Skip to content
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

Wait until all listeners are removed before finishing test #71

Closed
wants to merge 2 commits into from

Conversation

tschaub
Copy link
Contributor

@tschaub tschaub commented Feb 4, 2014

Without this change, running just the test/add_test.js tests, Node never exits. E.g.

./node_modules/grunt-contrib-nodeunit/node_modules/.bin/nodeunit test/add_test.js

I'm trying to test some changes and having trouble getting the tests to run properly. I'm guessing there may be other similar problems, but this one was the clearest.

Without this change, running just the test/add_test.js tests, Node never exits.  E.g.

    ./node_modules/grunt-contrib-nodeunit/node_modules/.bin/nodeunit test/add_test.js
This leaves the multi-watch tests as they were, but it makes the others consistent with regard to removing listeners.
@shama
Copy link
Owner

shama commented Feb 4, 2014

Thanks for the PR, give me a bit to merge as I want to move this into v0.5 branch as well. Have a look at the v0.5 branch. That is where the active development is currently at.

Which operating system and version of node are you running? Try npm test to run the test suite.

@tschaub
Copy link
Contributor Author

tschaub commented Feb 5, 2014

Looks like it is no longer an issue with that particular test file on your v0.5 branch. I'm running Node 0.10 on OSX (same behavior with 0.11). Looks like the keep alive timer was doing its job. You can still reproduce it with the v0.5 branch and the watch_test.js script:

./node_modules/grunt-contrib-nodeunit/node_modules/.bin/nodeunit test/watch_test.js

Without Grunt globally installed, npm test doesn't work. And I was trying to run just a single test file anyway. But it looks like you've addressed the issue I was trying to solve in the v0.5 branch.

@shama shama closed this in dbe8964 Feb 6, 2014
@tschaub tschaub deleted the remove-listeners branch February 6, 2014 18:27
shama pushed a commit that referenced this pull request Feb 17, 2014
.

Conflicts:

	test/add_test.js
	test/relative_test.js
	test/safewrite_test.js
shama pushed a commit that referenced this pull request Mar 20, 2014
.

Conflicts:

	test/add_test.js
	test/relative_test.js
	test/safewrite_test.js
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