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: remove flaky test functionality #812

Closed
wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Feb 12, 2015

Reverts nodejs/node-v0.x-archive#8689

I'm proposing that we remove this embarrassing functionality. If we need have special cases for tests then we should have that explicitly documented and implemented in the tests themselves.

@Fishrock123
Copy link
Contributor

+1

Any idea what sort of tests will fail after this?

@rvagg
Copy link
Member Author

rvagg commented Feb 12, 2015

I don't believe any will, we didn't merge in the list of flaky tests and went ahead and fixed those tests anyway

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/171/

@Fishrock123
Copy link
Contributor

Hmmm, some strange errors are cropping up on that CI run. Tested with & without the patch on OS X 10.10 and everything seems fine.

@jbergstroem
Copy link
Member

@Fishrock123 The add ons probably aren't build before run. Possibly separate test-addons from make test similar to make build-addons?

@rvagg
Copy link
Member Author

rvagg commented Feb 12, 2015

yes, I put test-ci back on instead of separate test-simple & test-message, so we can test #810

@bnoordhuis
Copy link
Member

LGTM

@jbergstroem
Copy link
Member

I'm all for removing the flaky stuff too.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 12, 2015

+1 and LGTM

@Fishrock123
Copy link
Contributor

Re-running the CI on this since some failures seemed unrelated: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/174/

@Fishrock123
Copy link
Contributor

Not sure if relevant, but this cropped up on iojs-centos5-64.

[01:47|%  36|+ 284|-   0]: release test-http-curl-chunk-problem 

=== release test-http-curl-chunk-problem ===
Path: parallel/test-http-curl-chunk-problem
dd command:  dd if=/dev/zero of="/home/iojs/build/workspace/iojs+any-pr+multi/nodes/iojs-centos5-64/test/tmp.0/big" bs=1024 count=10240
Server running at http://localhost:8080
making curl request
events.js:141
      throw er; // Unhandled 'error' event
            ^
Error: write after end
    at ServerResponse.OutgoingMessage.write (_http_outgoing.js:392:15)
    at Socket.<anonymous> (/home/iojs/build/workspace/iojs+any-pr+multi/nodes/iojs-centos5-64/test/parallel/test-http-curl-chunk-problem.js:48:9)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:166:7)
    at readableAddChunk (_stream_readable.js:146:16)
    at Socket.Readable.push (_stream_readable.js:109:10)
    at Pipe.onread (net.js:510:20)
Command: out/Release/iojs /home/iojs/build/workspace/iojs+any-pr+multi/nodes/iojs-centos5-64/test/parallel/test-http-curl-chunk-problem.js

@rvagg
Copy link
Member Author

rvagg commented Feb 14, 2015

@Fishrock123 already filed as #790 and dupe @ #809

rvagg added a commit that referenced this pull request Feb 16, 2015
Reverts nodejs/node-v0.x-archive#8689

PR-URL: #812
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@rvagg
Copy link
Member Author

rvagg commented Feb 16, 2015

landed via 20f8e7f

@rvagg rvagg closed this Feb 16, 2015
@rvagg rvagg deleted the remove-flaky-test-functionality branch February 16, 2015 04:29
@rvagg rvagg mentioned this pull request Feb 18, 2015
taylrj added a commit to taylrj/monorepo-template that referenced this pull request Nov 29, 2019
test something

PR-URL: nodejs/node#812
@jesec jesec mentioned this pull request Jul 4, 2023
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.

5 participants