Create first e2e tests for cert creation #28
Conversation
This makes it so that you don't have to wait to PR to see the github workflow, if you've created a new development branch to work on your feature.
Chromium tests are flaking pretty frequently. This will allow for less frequent manually rerunning while doing local e2e test runs.
I've added two basic tests: - Just create a cert and check that we end up on the right page - Create a cert and verify that the fields are filled out I've added some TODOs to verify even more fields than I've checked so far.
| /* Retry on CI only */ | ||
| retries: process.env.CI ? 2 : 0, | ||
| /* Retry on both CI and local dev */ | ||
| retries: 2, |
There was a problem hiding this comment.
Did you already notice brittleness or is this just in case?
There was a problem hiding this comment.
I've noticed, pretty frequently, the Chromium browser will just sit and timeout after 30 seconds waiting for the first page to load. Retries have made it work.
Not sure why; Firefox and Webkit have not experienced this.
|
Is the test ready for, uh, testing? ^.^ I’ve had a look over the code but haven’t run it locally yet. |
|
Yup, it's ready. You should see the tests running in the Github workflow, and they work locally for me. (Also I've "tested the tests" by modifying the assertions to incorrect values, seeing them fail, then switching back to the correct values and seeing them pass. You can try this exercise as well if you want more hands-on verification) |
|
Also, I recommend going into the playwright config locally and setting Headless to False; that'll let you see the test run in actual browser windows. |
|
I surmised that the server might not be ready right away when it starts listening to the port. With the I noticed another issue though (at least on my machine): The new docker-compose file is fairly inert apart from changing the port: The database seems to be the same. So if I run the tests and then resume my normal dev work, the test certs are in the database.
Thankies! |
|
Thanks for looking into this stuff; great ideas. Yep I can get on all this soon! Expect more additions to this PR within the next couple days. |
Now running the e2e tests locally won't modify your local im-app database anymore. They get their own im-app-test database.
Now, locally, you don't need to run `cp .env.test .env` anymore.
|
@Telofy These changes eliminate all sources of local conflict I could find (and also we don't need to I've tried a few scenarios running the dev server on 3000 while simultaneously running local e2e tests, and it all seems to go fine. |
|
Awesome! That all works now! It just looks like my fix didn’t fix the brittleness at all… I’ll look into it some more. |
|
Seems that the NEXTAUTH_URL variable is actually required: https://next-auth.js.org/warnings#nextauth_url |
|
Yeah, was a mistake to remove it… I’ve found a way to reference other variables in .env, so I’m going with that now to avoid the duplication. |
|
I’ve fixed my mistake, and I hope I’ve resolved the flakiness. I’ve switched to using the production build for the tests, which makes the start of the server quick. Other advantages are that it has sometimes detected errors for me that the type check couldn’t find and that didn’t come up in my tested through the dev server, and that the behavior is generally closer to production. But the disadvantage is that the startup time of the tests is long now. :-/ I also ran into this bug: microsoft/playwright#18865. Luckily fixed in the latest version. |
|
Awesome, great to have this merged, and thanks for the improvements! Ah yeah, it's a shame test startup will take longer now. Probably worth it though. |
I've created a couple basic e2e tests to test certificate creation in the UI.
Also a couple quality-of-life improvements related to tests and continuous integration.
See the individual commit messages for more details.
Critiques are extremely welcome; I'm still very new to Javascript and playwright!