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

feat(testing): add e2e test for 'Go Home' button #2648

Merged
merged 23 commits into from
Feb 25, 2021
Merged

feat(testing): add e2e test for 'Go Home' button #2648

merged 23 commits into from
Feb 25, 2021

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Jan 28, 2021 •
edited
Loading

This PR adds two e2e tests and makes a few modifications for testing.

Changes

  • adds a test for the login page
  • adds a test for the 'Go Home' button
  • update jest config testTimeout

Screenrecording

5029d1a9ccf52b90afb87b03f161dcde.mp4

Todos

  • fix Ci to not use container node
  • make test deterministic
  • make requested changes
  • update ignore paths for jest
  • fix .gitignore conflicts
  • fix test.sh script
  • try out setting cookie directly
  • double-check that we don't need to be setting --home
  • fix conflicts
  • update test after Vscode 1.52.1 #2543 is merged, see comment
  • refactor to promises and remove done (see comment from Anmol below)
  • address PR feedback
  • rebase
  • fix CI failures (check locally)
    • lint
    • fmt
    • release

@jsjoeio jsjoeio changed the base branch from master to add-playwright January 28, 2021 23:37
jsjoeio
jsjoeio commented Jan 28, 2021
View reviewed changes
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@jsjoeio jsjoeio marked this pull request as ready for review January 28, 2021 23:45
jsjoeio
jsjoeio commented Jan 28, 2021
View reviewed changes
test/goHome.test.ts Outdated Show resolved Hide resolved
code-asher
code-asher requested changes Jan 29, 2021
View reviewed changes
test/goHome.test.ts Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
test/login.test.ts Outdated Show resolved Hide resolved
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 29, 2021 •
edited
Loading

Something wrong in test. Forgot the $ in the env variable in the ci script

image

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 29, 2021

tests are passing locally but no in CI? so weird 🤔
image

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jan 29, 2021

I'm running into strange errors 🤔

image

nhooyr
nhooyr suggested changes Feb 1, 2021
View reviewed changes
test/goHome.test.ts Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
test/login.test.ts Outdated Show resolved Hide resolved
@jsjoeio jsjoeio marked this pull request as draft February 1, 2021 19:20
Base automatically changed from add-playwright to master February 2, 2021 18:02
@jsjoeio jsjoeio marked this pull request as ready for review February 2, 2021 22:07
@jsjoeio jsjoeio requested a review from nhooyr February 2, 2021 22:07
code-asher
code-asher reviewed Feb 3, 2021
View reviewed changes
ci/dev/test.sh Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
@jsjoeio jsjoeio marked this pull request as draft February 4, 2021 17:45
@jsjoeio jsjoeio self-assigned this Feb 4, 2021
@jsjoeio jsjoeio marked this pull request as ready for review February 4, 2021 21:59
nhooyr
nhooyr suggested changes Feb 5, 2021
View reviewed changes
src/common/util.ts Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
test/goHome.test.ts Outdated Show resolved Hide resolved
@jsjoeio jsjoeio modified the milestones: v3.8.1, v3.8.2 Feb 5, 2021
jsjoeio and others added 15 commits February 22, 2021 13:41
@jsjoeio
feat: add step to upload test videos
9e3c8bd
@jsjoeio
refactor: update test script to check env var
e077f2d
@jsjoeio
feat: add cookie utils for e2e tests
b02d2fb
@jsjoeio
refactor: manually add cookie goHome
2dc56ad
@jsjoeio
refactor: add note to test.sh about --home
d0eece3
@jsjoeio
refactor: update goHome location in test
06af8b3
@jsjoeio
refactor: use promises for goHome test
38d7718
@jsjoeio
refactor: create helpers.ts & add Cookie
3fa460c
@jsjoeio
chore: add todo regarding storage and cookies e2e
5857b25
@jsjoeio
refactor: add constants.ts with PASSWORD, etc
b0fd554
@jsjoeio
refactor(goHome): check url, remove timeout
d61bbc4
@code-asher @jsjoeio
Close context before browser
6d4f814 
This seems to resolve a warning about a process being forcefully exited.
@code-asher @jsjoeio
Fix unreadable wtfnode output
ef7e727
@code-asher @jsjoeio
Move wtfnode setup to global setup
6685b3a 
I think Jest provides separate console methods for each test so when the
socket tests finish Jest complains that a test keeps trying to output.
@code-asher @jsjoeio
Gate wtfnode behind WTF_NODE env var
47a05c9 
After thinking about it some more it's probably mostly only useful to
see the output when the tests are hanging. Otherwise there's a lot of
noise about Jest child processes and pipes.
@jsjoeio jsjoeio marked this pull request as ready for review February 23, 2021 00:14
@jsjoeio jsjoeio mentioned this pull request Feb 23, 2021
Merged
4 tasks
@jsjoeio jsjoeio requested review from a team and code-asher and removed request for oxy and code-asher February 24, 2021 23:23
code-asher
code-asher requested changes Feb 25, 2021
View reviewed changes
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Looks good to me after this change!

test/constants.ts Show resolved Hide resolved
@jsjoeio jsjoeio merged commit 7dbb72a into master Feb 25, 2021
@jsjoeio jsjoeio deleted the e2e-test-go-home branch February 25, 2021 18:21
@jsjoeio jsjoeio mentioned this pull request Feb 26, 2021
Closed
@jsjoeio jsjoeio added the testing Anything related to testing label May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhooyr nhooyr nhooyr approved these changes

@code-asher code-asher code-asher approved these changes

Assignees

@jsjoeio jsjoeio

Labels
testing Anything related to testing
Projects
None yet
Milestone
v3.9.1
Development

Successfully merging this pull request may close these issues.
3 participants
@jsjoeio @nhooyr @code-asher