test: move tick-processor tests to own directory#9506
Closed
Trott wants to merge 5 commits intonodejs:masterfrom
Closed
test: move tick-processor tests to own directory#9506Trott wants to merge 5 commits intonodejs:masterfrom
Trott wants to merge 5 commits intonodejs:masterfrom
Conversation
The tick-processor tests are inherently non-deterministic. They therefore have false negatives from time to time. They also sometimes leave extra processes running. Move them to their own directory until these issues are sorted. Note that this means that the tests will not be run in CI. Like the inspector tests and other tests, they will have to be run manually when they are wanted.
Trott
added
v8 engine
nodejs-github-bot
added
the
build
Member
Author
|
Fixes: #8725 sorta kinda I guess |
danbev
approved these changes
Nov 8, 2016
gibfahn
suggested changes
Nov 8, 2016
Member
There was a problem hiding this comment.
@Trott Could you add something to ./test/README.md documenting this?
Otherwise LGTM.
I feel that tests that aren't run as part of CI tend to stop being up to date, but I'm not sure what the best solution to that would be. Maybe a less frequently run ci target that runs everything?
Member
Author
|
@gibfahn OK, README info added. I included more details than we usually include based on text from @matthewloring so cc'ing him in the hope he can give it a quick review to confirm that I didn't introduce any errors in imprecision. |
matthewloring
approved these changes
Nov 8, 2016
test/README.md
Outdated
|
|
||
| Test configuration utility used by various test suites. | ||
|
|
||
| ### inspector |
There was a problem hiding this comment.
This should probably say tick processor
Member
Author
There was a problem hiding this comment.
Whoops! You are, of course, correct. Fixed. Thanks!
matthewloring
suggested changes
Nov 8, 2016
| // then a large number of unknown ticks should be present | ||
| base.runTest({ | ||
| pattern: /LazyCompile.*\[eval\]:1|.*% UNKNOWN/, | ||
| pattern: /LazyCompile.*\[eval]:1|.*% UNKNOWN/, |
Member
Author
There was a problem hiding this comment.
] does not need to be escaped in a regular expression unless it occurs in a character class. I've been removing unnecessary escaping (especially in regular expressions) when I touch a file lately. I can put it back if you or anyone else thinks the regular expression is easier to read with it.
There was a problem hiding this comment.
I personally find the explicit escaping more readable but I don't feel too strongly about it.
matthewloring
approved these changes
Nov 8, 2016
gibfahn
approved these changes
Nov 9, 2016
Member
Author
Trott
added a commit
to Trott/io.js
that referenced
this pull request
Nov 10, 2016
The tick-processor tests are inherently non-deterministic. They therefore have false negatives from time to time. They also sometimes leave extra processes running. Move them to their own directory until these issues are sorted. Note that this means that the tests will not be run in CI. Like the inspector tests and other tests, they will have to be run manually when they are wanted. PR-URL: nodejs#9506 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Matthew Loring <mattloring@google.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Member
Author
|
Landed in eed8b05 |
addaleax
pushed a commit
that referenced
this pull request
Nov 22, 2016
The tick-processor tests are inherently non-deterministic. They therefore have false negatives from time to time. They also sometimes leave extra processes running. Move them to their own directory until these issues are sorted. Note that this means that the tests will not be run in CI. Like the inspector tests and other tests, they will have to be run manually when they are wanted. PR-URL: #9506 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Matthew Loring <mattloring@google.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Dec 13, 2016
The tick-processor tests are inherently non-deterministic. They therefore have false negatives from time to time. They also sometimes leave extra processes running. Move them to their own directory until these issues are sorted. Note that this means that the tests will not be run in CI. Like the inspector tests and other tests, they will have to be run manually when they are wanted. PR-URL: #9506 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Matthew Loring <mattloring@google.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Contributor
|
hey @Trott this didn't land cleanly on |
MylesBorins
pushed a commit
that referenced
this pull request
Dec 21, 2016
The tick-processor tests are inherently non-deterministic. They therefore have false negatives from time to time. They also sometimes leave extra processes running. Move them to their own directory until these issues are sorted. Note that this means that the tests will not be run in CI. Like the inspector tests and other tests, they will have to be run manually when they are wanted. PR-URL: #9506 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Matthew Loring <mattloring@google.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
The tick-processor tests are inherently non-deterministic. They
therefore have false negatives from time to time. They also
sometimes leave extra processes running.
Move them to their own directory until these issues are sorted. Note
that this means that the tests will not be run in CI. Like the inspector
tests and other tests, they will have to be run manually when they are
wanted.
/cc @nodejs/build @nodejs/testing @matthewloring @indutny