Skip to content
This repository was archived by the owner on Sep 3, 2022. It is now read-only.

Conversation

dk1027
Copy link
Contributor

@dk1027 dk1027 commented Jun 15, 2020

Description

This PR updates test-e2e such that it loads analytics.js and test website from a local test server

  • The test website is adapted from www.library-test-site.com
  • loadAJS helper takes an option {local: bool}. When set to true, the test navigates to and fills in localhost:8000 on the test website settings to load AJS from the test server. In the future we could make this configurable between using local vs the live test site

This PR also refactored configuration into config.js (see comments).

The local test server expects analytics.js to be found in test-e2e/static, if it is not available, then it downloads it from the configured CDN under the specified test write key. This allows quick development iterations in the future where one make changes to analytics.js, move it to the static directory, and re-run the e2e tests.

Testing

Testing not required because this does not change analytics.js-core. Only test-e2e is updated.
Running make test-e2e

image

Release

New version is not required because this does not change the analytics.js library.

@@ -2,13 +2,12 @@ Feature('AJS Bundle');

const assert = require('assert');
const testSite = 'https://www.library-test-site.com';
const testWriteKey = 'TEWEu8XrcMVejk8GOulbEx7rHGyuuijV';
const testWriteKey = 'WJq9vAlUO5l2255jMg7eEthbkDtq1svu';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a different test project that uses default configurations. The other one has some integration(s) turned on using dummy settings.

@dk1027 dk1027 marked this pull request as ready for review June 15, 2020 23:40
@juliofarah
Copy link
Contributor

maybe we shouldn't add the original analytics.js file, but instead add instructions on how to generate it?

@dk1027 dk1027 force-pushed the dk1027/local-test-server branch from bd47995 to 113dc81 Compare June 17, 2020 23:43
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@9548983). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #155   +/-   ##
=========================================
  Coverage          ?   96.36%           
=========================================
  Files             ?        3           
  Lines             ?       55           
  Branches          ?        0           
=========================================
  Hits              ?       53           
  Misses            ?        2           
  Partials          ?        0           
Impacted Files Coverage Δ
lib/utils/map.js 100.00% <0.00%> (ø)
lib/utils/clone.js 100.00% <0.00%> (ø)
lib/utils/each.js 90.47% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9548983...113dc81. Read the comment docs.

@@ -0,0 +1,33 @@
// These config are used by tests and devServer.
Copy link
Contributor

Choose a reason for hiding this comment

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

should this file be .ts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to tackle this in a future PR because codeceptjs requires this config, and it is running as Javascript.

@dk1027 dk1027 changed the title Added local devServer. tests now defaults to use localhost:8000 and l… E2e tests now run against local dev server Jun 18, 2020
@dk1027 dk1027 merged commit 8d34568 into master Jun 18, 2020
@dk1027 dk1027 deleted the dk1027/local-test-server branch June 18, 2020 18:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants