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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: End to end tests #3450

Closed
wants to merge 9 commits into from
Closed

Conversation

yeraydiazdiaz
Copy link
Contributor

For discussion. A way of performing end to end or frontend only tests using Selenium and separate tests container. This allows fully testing features that would rely on a combination of template generation, pure JS and Stimulus code and potentially end to end feature critical flows.

Biggest caveat is of course needing a complete dev setup to run, which would specially noticable in CI since would require a whole make initdb. This could be less of a problem if we used a minimal SQL fixture.

I added a simple test around the password strength frontend verification in the register page as an example.

Thoughts? 馃

@yeraydiazdiaz yeraydiazdiaz force-pushed the frontend-tests branch 4 times, most recently from e33a643 to 30123c5 Compare March 29, 2018 14:17
@brainwane
Copy link
Contributor

@yeraydiazdiaz thank you so much for this prototype/WIP! As you saw in another comment, Dustin's been working on a different project this week. I would love thoughts from @ewdurbin if he has time in the next couple days (before he concentrates on a one-week programming retreat at the Recurse Center), and from @nlhkabu if she has time.

@yeraydiazdiaz
Copy link
Contributor Author

That's no problem @brainwane.

Just as an update the e2e tests pass in my local and generate a nice report on failure. I find it quite interesting to run them with local drivers and see the browser walking through the steps.

Travis is another story unfortunately. The environment builds not only the standard Node + Python env but also the Docker ones, resulting in ~12:30 minutes of build time, that's before failing for some reason when actually running the tests. Unfortunately I cannot debug the build since I don't have permissions, not quite sure what could be wrong.

@di
Copy link
Member

di commented Apr 17, 2018

Hey, this is pretty nice! I think it would be reasonable to create a fixture DB just for frontend-testing.

Unfortunately I cannot debug the build since I don't have permissions, not quite sure what could be wrong.

What permissions do you need to continue debugging?

@yeraydiazdiaz
Copy link
Contributor Author

I'd need to be able to use the "Debug build" function in Travis, not quite sure of the details behind it being available, sorry

@di
Copy link
Member

di commented Apr 18, 2018

@yeraydiazdiaz Can you check and see if you have the permissions you need now?

@yeraydiazdiaz
Copy link
Contributor Author

I can now restart the job, but cannot debug it, maybe it's because the job is fairly old. I'll restarting it and hopefully that does the trick. Thanks!

@yeraydiazdiaz
Copy link
Contributor Author

the job now fails for a different reason but the "Debug build" option is still not present 馃槙

@di
Copy link
Member

di commented Apr 18, 2018

@yeraydiazdiaz https://docs.travis-ci.com/user/running-build-in-debug-mode/ seems to indicate that the button might not be available to all projects, and that for a public repo, an API call is required. Could you try that instead?

@yeraydiazdiaz
Copy link
Contributor Author

@di I followed the guide but I'm getting a credentials error

$ curl -s -X POST \
  -H "Content-Type: application/json" \
  -H "Accept: application/json" \
  -H "Travis-API-Version: 3" \
  -H "Authorization: token <MY_TOKEN_FROM_PUBLIC_PROFILE>" \
  -d "{\"quiet\": true}" \
  https://api.travis-ci.org/job/359900199/debug

{
  "@type": "error",
  "error_type": "wrong_credentials",
  "error_message": "access denied"
}

I've tried logging in and out of Travis, but no luck

@di
Copy link
Member

di commented Apr 18, 2018

@yeraydiazdiaz Ah, looks like we need to contact support to enable the feature. I've emailed them, I'll update here when it's enabled.

@di
Copy link
Member

di commented Apr 18, 2018

@yeraydiazdiaz Sent you an email with some details, should be working now.

@yeraydiazdiaz
Copy link
Contributor Author

@di it's working now, thanks for sorting this out, I'll take a look at what's wrong now

@yeraydiazdiaz yeraydiazdiaz force-pushed the frontend-tests branch 2 times, most recently from 7934170 to 4f171d7 Compare April 19, 2018 12:48
@yeraydiazdiaz
Copy link
Contributor Author

@di finally managed to make the end to end tests pass after 14 min 40 sec

There's some things we could do to try and reduce the build time since there's no need to build the environment outside the containers for that particular build, but the install step is shared across them so I left it there for now.

Also running make tests_e2e will drop and reindex the DB which could be inconvenient while developing, maybe we could use a different DB name.

@yeraydiazdiaz
Copy link
Contributor Author

Potentially we could also take advantage of pytest-selenium鈥檚 support for Browserstack and run the frontend tests against supported browsers, although I haven鈥檛 tried it myself

@di
Copy link
Member

di commented Apr 26, 2018

@yeraydiazdiaz Nice work! Given that the tests only took about ~17 seconds to run, I'd say this would be merge-able if we could get that build time down a bit. Our goal should probably be something like 5 minutes if possible.

It looks like we're spending a fair amount of time unpacking, loading, migrating and indexing the development DB, so the first thing I think we should work on is producing a minimum-viable DB, perhaps with just one or two users and just a couple projects.

There's some things we could do to try and reduce the build time since there's no need to build the environment outside the containers for that particular build, but the install step is shared across them so I left it there for now.

I think we can override the general install by just providing some no-op command like install: : in the testing matrix.

Yeray Diaz Diaz and others added 8 commits April 28, 2018 10:36
Add selenium and a separate test container to invoking a specific
set of test designed for frontend. These can be extended to full
end to end if necessary.

Add make recipe `make tests_e2e` for invoking them.
Fix linting errors
Include e2e in travis conf
Add `mindb` to travis script
@yeraydiazdiaz yeraydiazdiaz force-pushed the frontend-tests branch 2 times, most recently from ab4277f to 4f8984c Compare April 28, 2018 10:14
@yeraydiazdiaz yeraydiazdiaz force-pushed the frontend-tests branch 5 times, most recently from 34811f9 to ab22d16 Compare April 28, 2018 14:48
@yeraydiazdiaz
Copy link
Contributor Author

@di thanks for the suggestion, it didn't occur to me to override install that's shaved roughly 3 minutes to 11 min 44 sec.

Regarding a minimal database setup, note that I've split part of the original initdb into a createdb recipe and tests_e2e actually calls a different mindb recipe which currently just creates the database, runs migrations and reindexes so it's completely empty.

The bulk of the time comes from pulling and building all the images. We could reduce this time further by creating a pre-built base image that we could reuse, either pushing it to a registry or caching it between Travis builds somehow.

If that's not an option I'm out of ideas to get to the ~5 minute goal.

@yeraydiazdiaz
Copy link
Contributor Author

I made an attempt of creating a base Docker image including the requirements in #3848. The idea was to create a base image tagged with a hash of the contents of requirements and package.json to a registry, then Travis could attempt to pull such image and skip the building process.

However, as described in #3848, Travis has a security restriction for projects that accept PRs from forked repositories where encrypted environment variables are deleted from the build to prevent malicious PRs from reading such environment variables.

I searched for other alternatives but seems they revolve around pushing the image to some repository which would have the same problem.

@yeraydiazdiaz
Copy link
Contributor Author

I think these are less important given we now have frontend unit tests. Closing.

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.

None yet

3 participants