Fix failing iframe e2e tests

[patcon]

Re-ticketed from https://github.com/pol-is/polis/issues/830#issuecomment-780342350

Expected behavior: All e2e tests should pass, all the time, on all CI and local workstations.

Actual behavior: Two e2e tests involving iframes fail on some systems:

System	Status
docker:local @ballPointPenguin	:x:
docker:local @micahstubbs	:x:
docker:digitalocean @patcon	❓
docker:ci GitHub Actions	✅

To Reproduce:

git clone https://github.com/pol-is/polis
cd polis
docker-compose up --build --detach
cd e2e
npm install
CYPRESS_BASE_URL=https://127.0.0.1.xip.io npm run e2e:standalone

Screenshots: None

Device information: None.

Additional context: See other thread linked at top.

Likely involves a timeout that isn't enough in some environments. Might involve capture of siteId from /integrate page, which maybe sometimes doesn't match for some reason TBD.

[patcon]

aaahhhh of course...

The test suite uses a custom version of embed.html, which allows passing in embed code dynamically from url: https://github.com/pol-is/polis/blob/4073e0d796405efff1bdc6479ce509b66decd224/.github/workflows/cypress-tests.yml#L59-L60

I chose to do this because i wasn't sure whether it would seem like an overzealous change to override the existing (static) embed.html file in order to start testing embeds. (I wasn't sure what the original embed.html was there for)

Fixes:

1. Just override client-admin/embed.html with our dynamic one (@pol-is/core ?), or
2. add the cp into a Makefile target and document it better in e2e test docs.

Thoughts @micahstubbs @ballPointPenguin?

To get passing tests

git clone https://github.com/pol-is/polis
cd polis

# NEW
cp e2e/cypress/fixtures/html/embed.html client-admin/embed.html

docker-compose up --build --detach
cd e2e
npm install
CYPRESS_BASE_URL=https://127.0.0.1.xip.io npm run e2e:standalone

[ballPointPenguin]

I wonder if there can be a slick way to skip the cp e2e/cypress/fixtures/html/embed.html client-admin/embed.html step.

I like to streamline these commands as much as possible. For example, here's how I run the tests (on dev):

git clone https://github.com/pol-is/polis
cd polis
docker-compose pull
docker-compose up -d
cd e2e
npm i
npm test

😉 see? nice and simple. I like it that way, as much as possible.

[patcon]

I personally love the idea of putting all of these in make commands :) Then we can just use them in npm run-scripts if we'd like? (I find using npm scripts directly are a bit annoying, as keeping them in JSON is annoying, e.g., chained with && operators and without ability to comment inline)

[patcon]

docker-compose pull will only work if the test environment is the same as the built containers, which seems not a constraint I'd expect to hold the testing process to? I dunno, assumption might work for a good while though

EDIT: e.g., as soon as we have feature toggles in config, which we already have for comment translation via google translate.

EDIT2: Mostly I don't run them locally, which is simplest :) And when we do run them locally, it will very rarely be on the built ones, but rather on a feature branch, right? This "I just want to run it on dev branch" thing we're doing right now is kinda like a rare due diligence thing, no? But normally running against pulled images isn't really practical to optimize for, as they'll have been tested by CI already (and in the PR).

[ballPointPenguin]

docker-compose pull will only work if the test environment is the same as the built containers, which seems not a constraint I'd expect to hold the testing process to?

agreed. I should clarify:

Testing the tests (ie sanity check that the current tests pass as is): docker-compose pull

Testing anything else (e.g. local changes, feature branch, newly written tests etc): docker-compose --build

[ballPointPenguin]

But as a sanity check, I do believe it is desirable for a new developer or whomever to run something very simple to just run the tests and get a passing build, even if using the pulled containers (which are 100x faster than building from scratch). This is helpful for troubleshooting one's environment and just knowing that they have a sane green baseline from which to begin.

[ballPointPenguin]

Another scenario would be if the build fails on CI. The first thing I'd want to do is pull the built containers and run the test suite to see if it also fails on my machine and proceed from there.

edit: This would only apply if the build fails in dev, ie after a merge. Maybe in the future we will have built containers for specific branches or PRs to pull.

[patcon]

This is helpful for troubleshooting one's environment and just knowing that they have a sane green baseline from which to begin.

You're totally right, good call :) I can imagine a section of readme in Contributing section, like "So you want to write/run an e2e test? Ok, start here <baby step of quickly running known good state>"

[patcon]

The first thing I'd want to do is pull the built containers and run the test suite to see if it also fails on my machine and proceed from there.

My first step right now is to click "re-run all" in the checks screen, as more often than not that resolves things, at least with our current e2e test shakiness. But that's a privilege of someone with push access.

Wonder if there's a GitHub app to allow a view with non-push perms to do that...

[patcon]

https://github.com/marketplace/actions/re-run-workflow The above GitHub Action allows designating a label (e.g., "rerun-tests") that, when applied, will re-run the tests and then remove the label. This allows anyone with triage permission to re-run failed tests (normally only available to push permission folks)

[ballPointPenguin]

This is complete