Skip to content

feat(micro): fix snowplow micro test suite#11

Open
E-Rick wants to merge 33 commits intomainfrom
feature-snowplow-micro-test-suite
Open

feat(micro): fix snowplow micro test suite#11
E-Rick wants to merge 33 commits intomainfrom
feature-snowplow-micro-test-suite

Conversation

@E-Rick
Copy link
Contributor

@E-Rick E-Rick commented Jul 19, 2021

Changes proposed in this Pull Request:

Fixes the issues with running the Snowplow Micro test suite. Removed checks for schema data such as articleIds because of trouble with pulling the attributes async from the Nightwatch API.

This is moved over from Snowplow repo and requires you to install localtunnel globally right now because I haven't got it to work within the test suite yet. So it's a manual process of starting the tunnel in a separate terminal window to get the HTTPS going.

How to test the changes in this Pull Request:

  1. npm install -g localtunnel
  2. docker compose up -d to start running the micro container
  3. lt --port 9090 --subdomain lnlmicro --open to run the localtunnel server for https
  4. Click okay in the browser
  5. run nightwatch to run the default env
  6. nightwatch -e prod to run prod tests

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@E-Rick E-Rick added bug Something isn't working enhancement New feature or request labels Jul 19, 2021
@E-Rick E-Rick self-assigned this Jul 19, 2021
@E-Rick E-Rick requested a review from tngzng July 28, 2021 21:59
@E-Rick E-Rick changed the title Feature snowplow micro test suite feat(micro): fix snowplow micro test suite Jul 28, 2021
Copy link

@tngzng tngzng left a comment

Choose a reason for hiding this comment

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

comments

looks good!

the two-step test for each module looks good for now (widget seen, widget clicked) and i'm excited to test out the github workflows in action

local testing

the command docker compose up -d fails for me with Error response from daemon: invalid mount config for type "bind": bind source path does not exist: /Users/ting/lnl/wp-content

do you mind updating the local testing steps in the pr description so i can try it out again?

suggestions

in the future, i might suggest submitting separate pr's at each of these steps if possible:
1 - direct copy of code from another repo (same goes for any scaffolding from a template)
2 - addition of new code
3 - addition of github action workflow config

we can chat more at our next one on one!

Comment on lines +15 to +16
matrix:
python-version: [3.8]
Copy link

Choose a reason for hiding this comment

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

do we need to use the matrix strategy if we're not using multiple configuration options?

from the docs

You can specify a matrix by supplying an array for the configuration options. For example, if the runner supports Node.js versions 10, 12, and 14, you could specify an array of those versions in the matrix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I added it at the time to be flexible, but you are right it makes more sense to only add what is needed.

I'll change it 🙌🏽

matrix:
python-version: [3.8]

steps:
Copy link

Choose a reason for hiding this comment

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

very clear workflow steps!

},
{
"name": "Washington City Paper - Dev",
"priority": 9,
Copy link

Choose a reason for hiding this comment

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

curious what this priority num means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enrichment process reads through each object depending on priority. Lower number = higher priority.

Priority Helps the schema resolver determine the lookup order for the schema repositories. See` Iglu Schema Resolution

'Testing micro received the expected "widget_seen" event with context'
);
},
'Step 2: Number of good events after user CLICKS a RECENT POST is equal to TWO': async function (
Copy link

Choose a reason for hiding this comment

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

these descriptions are helpful. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants