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

Changing root script for tests under Node. #1837

Merged
merged 4 commits into from
May 2, 2018

Conversation

AnmAtAnm
Copy link
Contributor

@AnmAtAnm AnmAtAnm commented May 2, 2018

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Proposed Changes

Changing root script for tests under Node.

Was: tests/jsunit/test_runner.js
Now: tests/test_runner.js

The webdriver & jsunit calls formerly in tests/jsunit/test_runner.js are now in function runJsUnitTestsInBrowser(), in run_jsunit_tests_in_browser.js, called from tests/test_runner.js.

Reason for Changes

This makes room for additional tests under Node, such as headless and generator tests.

Test Coverage

ran npm test from the root, ensuring the previous tests run exactly as before.

Was: tests/jsunit/test_runner.js
Now: tests/test_runner.js

The webdriver & jsunit calls formerly in tests/jsunit/test_runner.js
are now in function runJsUnitTestsInBrowser, in
run_jsunit_tests_in_browser.js, called from tests/test_runner.js.

This makes room for additional tests under Node, such as a headless.
@rachel-fenichel
Copy link
Collaborator

Looks fine but lint is failing, probably because this file is no longer excluded. Add it to .eslintignore and you're good to go.

@AnmAtAnm
Copy link
Contributor Author

AnmAtAnm commented May 2, 2018

Fixed lint via .eslintignore. In the future, we'll probably want a solution that allows us to list Node scripts like this.

@AnmAtAnm AnmAtAnm merged commit 0ffd5dd into google:develop May 2, 2018
@AnmAtAnm AnmAtAnm deleted the node-test-runner branch May 15, 2018 16:58
This was referenced Jun 26, 2018
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.

2 participants