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

test: fix flaky test by removing timer #9199

Merged
merged 1 commit into from
Oct 25, 2016
Merged

Conversation

evanlucas
Copy link
Contributor

@evanlucas evanlucas commented Oct 20, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

This fixes one of the tests that has been failing on CI on freebsd for
a bit by removing an unnecessary timer.

Fixes: #7929

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 20, 2016
@mscdex mscdex added the dgram Issues and PRs related to the dgram subsystem / UDP. label Oct 20, 2016
@Trott
Copy link
Member

Trott commented Oct 20, 2016

LGTM, #9197 is basically the same fix, doesn't matter much which one lands, but one of them should. :-D

@evanlucas
Copy link
Contributor Author

@Trott I'm pretty sure they both need to land. Those are two different files (although pretty similar).

@Trott
Copy link
Member

Trott commented Oct 20, 2016

@evanlucas Needless to say (but I'm going to say it anyway), you are correct!

@@ -11,12 +11,7 @@ const buf = Buffer.allocUnsafe(256);
const onMessage = common.mustCall(function(err, bytes) {
assert.strictEqual(err, null);
assert.equal(bytes, buf.length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: While we're in here anyway, maybe change this to assert.strictEqual()?

@@ -11,12 +11,7 @@ const buf = Buffer.allocUnsafe(256);
const onMessage = common.mustCall(function(err, bytes) {
assert.strictEqual(err, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: assert.ifError()?

This fixes one of the tests that has been failing on CI on freebsd for
a bit by removing an unnecessary timer.

PR-URL: nodejs#9199
Fixes: nodejs#7929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@evanlucas evanlucas closed this Oct 25, 2016
@evanlucas evanlucas deleted the flakytest branch October 25, 2016 13:20
@evanlucas
Copy link
Contributor Author

Landed in 2a654e4. Thanks!

@evanlucas evanlucas merged commit 2a654e4 into nodejs:master Oct 25, 2016
evanlucas added a commit that referenced this pull request Nov 2, 2016
This fixes one of the tests that has been failing on CI on freebsd for
a bit by removing an unnecessary timer.

PR-URL: #9199
Fixes: #7929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 18, 2016
This fixes one of the tests that has been failing on CI on freebsd for
a bit by removing an unnecessary timer.

PR-URL: #9199
Fixes: #7929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 19, 2016
This fixes one of the tests that has been failing on CI on freebsd for
a bit by removing an unnecessary timer.

PR-URL: #9199
Fixes: #7929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate flaky test-dgram-send-callback-buffer on FreeBSD
8 participants