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

Running query tests in browser #8584

Merged
merged 62 commits into from
Oct 10, 2019
Merged

Running query tests in browser #8584

merged 62 commits into from
Oct 10, 2019

Conversation

arindam1993
Copy link
Contributor

@arindam1993 arindam1993 commented Aug 2, 2019

This PR adds a bunch of new tooling to allow the query tests to be run in the browser.
New tools added:

  • testem: this is a test-runner that that handles
    • launching browsers
    • serving test files to launched browsers, and receiving test results back via websocket
    • persisting test results to disk once tests have finished in xunit format
  • tape: browser compatible version of tap, used to write the tests.
  • browserify: tape is not browser compatible ootb, so we need to build it ourselves, and rollup does not handle tape's commonjs style well and fails at trying to inject node globals and builtins. So we build it with browserify.

Architecture:

1.Loading fixture data to the browser

This is done via a preprocessor that generates fixtures.json file within test/integration/dist that contains all the data within the directory inlined within it, this file can then be imported into the test root file and the rollup will inline everything so all the data is available synchronously.

2.Bundling resources for the browser

There are 3 bundles that get built in parallel before the test suite starts
-tape_config.js: Done via browserify, and exposed as a global tape.

  • mapboxgl: Done via rollup, exposed as a global mapboxgl.
  • query-browser.js: this is the entrypoint of the test suite, built via rollupas an iife so that tests start executing as soon as the script is loaded.

3. Test running

testem provides hooks to run custom code before and after tests. The before_tests hook is used to run the build, and start an http server thats used to serve up certain resources such as spritesheets and fonts( this is the same as the existing implementation).
testem then launches its own server with a static html page which server up the built javascript resources. An extra script injected by testem onto the page captures the tap formatted console.log messages and relays them over websocket to be interpreted on the server to detect when the test suite has finished.
It then converts the gathered tap formatted report and convertes it to xunit format for integration with CircleCI's test reporting.

4.Test reporting

testem finishes running tests and persists the test report in xunit format to disk.
I've intentionally left a failing query test so we can see how it shows up in circle.

Guide:

Adds a bunch of npm scripts:
yarn run test-query-browser runs all the tests in headless chrome and exits
yarn run watch-query starts build watchers and launches a chrome window to run tests so you can work iterate on tests.
yarn run start-query-debug starts both the test server and the debug server while sharing the saem build-watcher.

  • briefly describe the changes in this PR

@arindam1993
Copy link
Contributor Author

Would be good to add flow typing to the new test lib scripts.

Unfortunately testem does not play nice with esm, and flow for commonjs modules is not great.

I found a couple issues with running the testem server locally and trying to build new query tests or modify code to fix test output. Looks like you are missing watchers for the test fixture and gl-js targets.

Addtionally, if a browser stays open to the testem page, it holds on to the page (and reruns it) even if the testem server is re-launched in the terminal. This gives a false sense of whether the old/new code is being run.

I added the watchers you can now do.
yarn run watch-query: runs only the query server in watch mode
yarn run watch-query-debug: runs both the query and the debug server working off the same gl-js build.

testem.js Outdated
@@ -0,0 +1,66 @@
/* eslint-disable import/no-commonjs */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this file need to be in the root directory? If not, let's move it to somewhere in the /test/* directory hierarchy to avoid clutter.

- v2-yarn-{{ checksum "yarn.lock" }}
- run: yarn
- v3-yarn-{{ checksum "yarn.lock" }}
- run: sudo npm install -g yarn & yarn install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why sudo? Docker builds typically run as the root user anyway.

Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nearly ready. A few items to wrap up before merging:

  • Update test-query and related package.json scripts to use this version of the test instead of the node based tests
  • Update integration tests readme with instructions to run the node-based tests, and to run/debug the testem based tests.

@@ -100,7 +100,7 @@ jobs:
- restore_cache:
keys:
- v3-yarn-{{ checksum "yarn.lock" }}
- run: sudo npm install -g yarn & yarn install
- run: npm install -g yarn & yarn install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs a && to serialize the steps. A Single & will fork and try to run it in the background

Arindam Bose added 3 commits September 23, 2019 14:30
- update debug pages to have a more reliable part to `access_token_generated.js`
- Update npm run scripts for query tests to use browser version by default.
@arindam1993
Copy link
Contributor Author

This looks nearly ready. A few items to wrap up before merging:

  • Update test-query and related package.json scripts to use this version of the test instead of the node based tests
  • Update integration tests readme with instructions to run the node-based tests, and to run/debug the testem based tests.

Done 👍 , I also made it so that it starts the testem server when running start/start-debug both the debug pages on :9966/debug and the test page on :7357 run off the same bundle and build watcher.

… process to prevent terminal from getting messed up.
Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Arindam Bose added 2 commits October 1, 2019 13:14
…est-browserify

# Conflicts:
#	package.json
#	yarn.lock
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.

4 participants