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

Enable single test runs on referrer trimming #96

Merged
merged 6 commits into from
Aug 31, 2022

Conversation

SlayterDev
Copy link
Contributor

This PR passes around the testid as well as adds manual test links to run single tests on this page.

@SlayterDev SlayterDev requested a review from kdzwinel August 29, 2022 20:09
Copy link
Member

@kdzwinel kdzwinel left a comment

Choose a reason for hiding this comment

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

I understand why we want to have those shortcuts in the HTML, but why do we need to pass this ID around is unclear to me? We get testid from the url param, we pass it to the test, it ends up in the url to the come-back page, gets passed back by the come-back page in the url - and then I'm not sure what happens?

@SlayterDev
Copy link
Contributor Author

When the test page runs the test it:

  • Checks if the test results are stored locally and adds them to the page
  • checks if the results for the last test are in the url and puts them in the results section in the page
  • otherwise runs the test

It does this for each test in the array. Currently if we pass a testid when we run the test we get the filtered array which calls the specified test. When the page comes back with the results we've lost the id so it operates on the full array of tests which puts the last run in the wrong spot and continues with the others.

With this change passing along the testid, when we come back to the results page the testid will ensure the array of tests is filtered again and will stop with just that one test instead of continuing with the others.

Copy link
Member

@kdzwinel kdzwinel left a comment

Choose a reason for hiding this comment

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

Gotcha, that makes sense, thanks for explanation!

@kdzwinel kdzwinel merged commit e88605d into main Aug 31, 2022
@kdzwinel kdzwinel deleted the brad/referrer-test-delay branch August 31, 2022 10:28
kdzwinel added a commit that referenced this pull request Aug 31, 2022
kdzwinel added a commit that referenced this pull request Aug 31, 2022
@kdzwinel kdzwinel restored the brad/referrer-test-delay branch August 31, 2022 10:34
@kdzwinel
Copy link
Member

Sorry, I had to revert it because it was breaking the main functionality of "start test". The way I tested (Safari):

  • I've run couple of manual tests
  • I've tried running the "Start test" (with storage cleared and clean url)

Expected: full test runs normally
Actual: only one test runs and no results are displayed

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