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.

Sign up for GitHub

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

Jump to bottom

Fix "I wait _ seconds": Allow that step to work before an interview has been loaded #387

Closed
plocket opened this issue Nov 17, 2021 · 4 comments · Fixed by #459
Closed

Fix "I wait _ seconds": Allow that step to work before an interview has been loaded #387

plocket opened this issue Nov 17, 2021 · 4 comments · Fixed by #459
Assignees
@plocket
Labels
bug Something isn't working priority A combination of urgency and impact

Comments

@plocket
Copy link
Collaborator

plocket commented Nov 17, 2021

That step (correctly) uses our afterStep code to complete what it's doing. We need to configure afterStep to handle situations where no page has yet been loaded so that it can be used even when interviews have not yet been loaded so that someone can wait before an interview loads.

This is actually a hack for dealing race conditions for the first test of the test suite. Right now, we're having trouble with server load times after the repo has been pulled into a project - the server often still hasn't completely loaded and our load tests aren't sufficient for that. That's a whole other complicated discussion. This fix might also be useful under other circumstances, though I don't know what those would be.
@plocket plocket added bug Something isn't working priority A combination of urgency and impact labels Dec 10, 2021
@plocket
Copy link
Collaborator Author

plocket commented Dec 28, 2021 •
edited

At least one of our assembly_line.feature file tests uses that as the first step now, so this must be fixed. Not sure when that happened. We should add a formal test for it in 'establishing_steps.feature'.

@plocket plocket closed this as completed Dec 28, 2021
@plocket plocket mentioned this issue Dec 28, 2021
Closed
@plocket
Copy link
Collaborator Author

plocket commented Dec 28, 2021

Dang, got mixed up between this and the custom timeout step. This one is the 'wait' Step, which may still need work.

@plocket plocket reopened this Dec 28, 2021
@plocket
Copy link
Collaborator Author

plocket commented Dec 28, 2021

For v4, there's a 'wait'-type function in its own file. Maybe we should make something for v3 as well. For now we'll add it separately and reduce duplication later.

@plocket plocket self-assigned this Dec 28, 2021
plocket added a commit that referenced this issue Dec 28, 2021
@plocket
Allow a developer to wait as a first Step. Close #387. Add test.
c9e879b
@plocket plocket mentioned this issue Dec 28, 2021
Merged
rpigneri-vol pushed a commit that referenced this issue Feb 7, 2022
@plocket
Allow a developer to wait as a first Step for v3. Address #387. Add t…
fdae93a 
…est. (#459)

This adds a temporary function for waiting in scope.js that will soon be replaced by a singular function in the utils folder. That said, that function is part of v4 changes, and this can improve v3 as well, so we might as well not wait.

Addresses #387, after a little confusion. Should add this to the testing docs as a 'first step' after this is merged.

[Also, generally adds safety measures for when page does not exist.]

------------------

* Allow a developer to wait as a first Step. Close #387. Add test.

* Offer proper fallback for wait options
plocket added a commit that referenced this issue Feb 8, 2022
@plocket
Allow a developer to wait as a first Step v4. #387. Add test.
09704e8 
Will be able to close once we've added this as an establishing
step (in addition to it being a regular step).

Also, generally adds safety measures for when page does not exist.
@plocket plocket mentioned this issue Feb 18, 2022
Merged
plocket added a commit that referenced this issue Feb 18, 2022
@plocket @BryceStevenWilley
Allow a developer to wait as a first Step v4. #387. Add test. (#506)
a7c2ec7 
Closes #387.

Also, generally adds safety measures for when page does not exist.

Very similar to PR #459, but moving the responsibility down to a spot that most functions make use of, meaning that it'll be applied to a lot more cases. They mostly won't need it, but it might still be worth being more comprehensive.

* Allow a developer to wait as a first Step v4. #387. Add test.

Will be able to close once we've added this as an establishing
step (in addition to it being a regular step).

Also, generally adds safety measures for when page does not exist.

* Add test

Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com>

Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com>
@plocket
Copy link
Collaborator Author

plocket commented Feb 18, 2022

Closed by #506

@plocket plocket closed this as completed Feb 18, 2022
plocket added a commit that referenced this issue Mar 2, 2022
@mcdonaldcarolyn @plocket @BryceStevenWilley
closes #466 add log file v4 (#517)
7328c84 
Created log.txt and git hub artifact for reports. Closes #466.

* add log.txt for report messages

* create artifact for logs

* Update changelog

* Update our package's dependencies for v4 (#503)

I'm thinking this is just going to be for v4. Not bothering with this for v3 unless we absolutely have to since none of the vulnerabilities are severe. My current rationale is that the more work we do to maintain 3, the less work we can do getting v4 ready for release. Ready to hear opinions.

- Close #164, update cucumber to v7
- Prepare for v8 of cucumber because I won't remember it later
- Close #394, update puppeteer
- Update our version of node (and that of our action that we'll run for other people's libs). [Addresses #393 so we can use the suffolk npm org package.]
- Use `npm audit` to fix the remaining vulnerabilities (now 0)
- [Remove package.json as discussed in #489 to align our tests' behaviors with those of our users.]

* Update action.yml node to v17

* Update from cucumber v6 to v7. See details.

See https://github.com/cucumber/cucumber-js/blob/main/docs/migration.md#migrating-to-cucumber-js-7xx

Only use cucumber setDefaultTimeout globally and use a shim that replicates the fix in v8 that lets you do custom timeouts more easily so we can still give enough time for steps that may need more time.

Use all caps for statuses.

Test screenshot step.

Btw, the cucumber test output visually looks a bit different now - when a scenario passes, all the steps pass too.

Sorry about the little comment changes, etc. Tried to remove a lot of those incidental unrelated changes.

* Update puppeteer to latest (13). See details below.

- page.waitFor -> page.waitForTimeout and page.waitForSelector (Got deprication notice. See puppeteer/puppeteer#6214.)
- remove removeEventListener (we'd need to change it to removeListener anyway - v4.0.0 and see https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#eventemitterremovelistenerevent-handler). For now we'll count on page close taking care of it, just in case removing it would prevent multiple-file-downloads.

* Update GitHub worflow node version, tweak changelog item order

* Fix npm audit vulnerabilities and update action.yml cucumber

* Pin the colors lib in action.yml

* Remove package-lock.json #489, use kiln v4 for users, CHANGELOG

* Fix custom timeout, remove duplicate report entry, as per review

* Allow a developer to wait as a first Step v4. #387. Add test. (#506)

Closes #387.

Also, generally adds safety measures for when page does not exist.

Very similar to PR #459, but moving the responsibility down to a spot that most functions make use of, meaning that it'll be applied to a lot more cases. They mostly won't need it, but it might still be worth being more comprehensive.

* Allow a developer to wait as a first Step v4. #387. Add test.

Will be able to close once we've added this as an establishing
step (in addition to it being a regular step).

Also, generally adds safety measures for when page does not exist.

* Add test

Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com>

Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com>

* add log to gitnore and cleanup console.logs and typos

* add empty string to file

Co-authored-by: plocket <52798256+plocket@users.noreply.github.com>
Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@plocket plocket

Labels
bug Something isn't working priority A combination of urgency and impact
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow a developer to wait as a first Step for v3. Address #387. Add test. SuffolkLITLab/ALKiln
1 participant
@plocket