This repository was archived by the owner on Oct 15, 2020. It is now read-only.
test: skip tests that are marked flaky for node-chakracore #436
Conversation
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
Remove mentions to tests that no longer exist from .status files.
Tests were marked as FLAKY in the .status files for ChakraCore because they were failing. This commit changes the .status files to skip those tests instead. Fixes: nodejs#426
Some tests check for the ChakraCore engine at runtime to skip, since these tests depend on V8 features. This commit adds those tests to the .status files, so they aren't started at all.
Move all tests that are fully skipped at runtime for the ChakraCore engine to the .status files as a SKIP entry, so they aren't started at all.
joaocgreis
approved these changes
Nov 29, 2017
Member
joaocgreis
left a comment
joaocgreis
left a comment
There was a problem hiding this comment.
LGTM, cc @nodejs/node-chakracore
With this we lose the ability to quickly run these tests, but we reduce the delta to the upstream tests and almost everything is organized in the status files. Almost, because tests that are only partially skipped must still be skipped in the test file with common.skip. I think this is a good solution.
jackhorton
approved these changes
Nov 29, 2017
Contributor
jackhorton
left a comment
jackhorton
left a comment
There was a problem hiding this comment.
LGTM, pending CI: https://ci.nodejs.org/job/chakracore-test-pull-request/161/
Contributor
|
@MSLaguana you had seen that lint error before, no? |
Contributor
|
Yeah, we haven't spent the time to make our readme files lint-clean since that's a relatively new addition. |
joaocgreis
pushed a commit
that referenced
this pull request
Dec 6, 2017
Remove mentions to tests that no longer exist from .status files. PR-URL: #436 Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Jack Horton <jahorto@microsoft.com>
joaocgreis
pushed a commit
that referenced
this pull request
Dec 6, 2017
Some tests check for the ChakraCore engine at runtime to skip, since these tests depend on V8 features. This commit adds those tests to the .status files, so they aren't started at all. PR-URL: #436 Refs: #426 Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Jack Horton <jahorto@microsoft.com>
Member
|
Landed in a7d03f2...a008278. Thanks! |
kfarnung
pushed a commit
to kfarnung/node-chakracore
that referenced
this pull request
Jan 11, 2018
Tests were marked as FLAKY in the .status files for ChakraCore because they were failing. This commit changes the .status files to skip those tests instead. PR-URL: nodejs#436 Fixes: nodejs#426 Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Jack Horton <jahorto@microsoft.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
Currently, most tests that are marked as
FLAKYfor theChakraCoreengine in the tests'.statusfiles are actually failing everytime they are run.This PR marks those tests as
SKIP, instead.FLAKYshould be used for tests that fail only sometimes due to timing or racing conditions issues but pass most of the time.This PR contains four different changes, divided by commit:
.statusfiles, mentions to tests that no longer exist.FLAKYfor Node-Chakracore in the.statusfiles..statusfiles, tests that are being skipped at runtime for depending on V8 features..statusfiles.Fixes: #426
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test