WIP - add JS functional test harness #2
Merged
Conversation
This file contains 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
biancadanforth
force-pushed
the
122-js-test-harness
branch
7 times, most recently
from
e99b94f to
8711868
Compare
biancadanforth
force-pushed
the
122-js-test-harness
branch
4 times, most recently
from
34c04b0 to
3ef19ca
Compare
|
Rebased this branch off of the |
biancadanforth
force-pushed
the
122-js-test-harness
branch
17 times, most recently
from
1ee42b0 to
4e265b1
Compare
biancadanforth
force-pushed
the
122-js-test-harness
branch
4 times, most recently
from
ef95292 to
f9bb999
Compare
Now that we are actually linting the isVisible.js functional test, eslint reports an error: ``` /home/travis/build/mozilla/fathom/test/functional/isVisible.js 4:68 error "../../utilsForFrontend" is not found node/no-missing-require ``` I believe this is because 'require' is relative to the directory in which the script is found, while eslint is checking relative to the current working directory. This could be resolved by: * Ignoring the error (easiest) * Updating 'eslint-plugin-node' to >= v5.1.0 in which we can apply the 'resolvePaths' setting. * This requires also updating 'eslint', as otherwise 'make lint' fails * Updating 'eslint' updates default rules, for which there are then dozens of new violations throughout the source code to address * I vote that considering updating eslint be a follow-up issue, rather than part of fixing mozilla#122 In light of these options, I chose the best, easiest one, since it doesn't blow up everything else.
biancadanforth
force-pushed
the
122-js-test-harness
branch
from
f9bb999 to
68b791b
Compare
biancadanforth
force-pushed
the
122-js-test-harness
branch
12 times, most recently
from
6e169b6 to
4c69f81
Compare
Added a timeout to the 'after all' hook, as it is possible this can take longer than two seconds. Also made the callbacks into 'it' regular functions instead of arrow functions for consistency with the 'after all' hook. * With arrow functions, the 'this' binding for 'this.timeout' does not point to the desired target, and the 'after' hook threw an error if I tried to use the original approach I had with the 'it' blocks.
biancadanforth
force-pushed
the
122-js-test-harness
branch
4 times, most recently
from
e3bc99a to
3bbbb34
Compare
Add '/* istanbul ignore next */' to stop trying to cover the following methods, since they are executed in the 'isVisible.html' test page, and that scope does not have coverage variables defined. * ancestors * isDomElement * isVisible * toDomElement This allows coverage to continue to work (ignoring these functions) and keeps the JS functional tests in the same stage as the rest of the JavaScript tests in TravisCI. It should be noted that none of these functions previously had test coverage, which is part of why this approach is feasible.
Move the set up and tear down commands inside of 'npm test' instead of just in '.travis.yml'.
biancadanforth
force-pushed
the
122-js-test-harness
branch
from
0f10f12 to
b04d519
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.