Skip to content

Make it possible to test node_redis in a Docker environment #788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 25, 2015

Conversation

Starefossen
Copy link
Contributor

There is an ongoing effort in the official Node.JS Build and Test Working Group nodejs/smoke-test#1 to be able to test popular NPM packages as a part of the official test suite and continuous integration for Node.JS. The goal here is to more quickly identify what breaks when, giving core developers important feedback on new features, and package authors an early warning and time to roll out new compatible versions with new Node.JS releases. We believe this effort will contribute to make the overall experience of using Node.JS better.

Testing drivers are particularly challenging; but equally important, since they rely on external dependencies that Node.JS does not control. We are proposing a test infrastructure composed of Docker containers which allows us to create all the different external dependencies in a easy, reproducible, and secure way; you can follow that effort here nodejs/smoke-test#3.

What we need from the drivers is that they 1) do not assume that the test suite is running on the same machine as the services they are dependent upon; 2) that we can in some way configure host and corresponding port variables to point to the external service.

When we run the node_redis test suite in the following way:

docker run --link redis node:latest npm test

… Docker creates two environment variables to the linked Redis service:

  • REDIS_PORT_6379_TCP_ADDR - address of the linked redis service
  • REDIS_PORT_6379_TCP_PORT - port of the linked redis service

I have made three modifications to the test suite to get it working with the Docker setup described above:

  1. Checking for REDIS_PORT_6379_TCP_ADDR and REDIS_PORT_6379_TCP_PORT before setting default values to HOST and PORT variables (d34dcc8) (da535a6)
  2. Always pass HOST and PORT variables when creating new redis clients in the test suite to prevent clients from connecting to 127.0.0.1 which seams to be the default if no host is provided (8b5ce10)

@Starefossen Starefossen changed the title Enable docker testing Make it possible to test node_redis in a Docker environment Jul 24, 2015
@erinishimoticha
Copy link
Contributor

This looks good to me! We'll have to reimplement it in the new mocha test suite that we're working on, but I don't see any reason why this shouldn't be merged in the interim.

@Starefossen
Copy link
Contributor Author

Thats great @erinspice! I'll be happy to help out with implementing this into the new test suite, as you can see it was not very many changes. The new test suite is in the mocha-test-suite-no-grunt branch right?

I reverted the skip SOCKET test as I found a reliable way to share the Redis unix socket between two containers. So this change should be good to merge.

@erinishimoticha
Copy link
Contributor

That's right, that's the right branch.

erinishimoticha added a commit that referenced this pull request Jul 25, 2015
Make it possible to test node_redis in a Docker environment
@erinishimoticha erinishimoticha merged commit b53ebb2 into redis:master Jul 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants