Improve E2e test documentation

[atrevino89]

Description

There's no issue related to these changes and actually not sure if there's a need for this but after not working on anything related to LiteFarm for more than a year I wanted to come back with some ideas where my expertise can be used for the cause.

I wanted to execute the E2e tests because for me its easier to take a tour on the app like that as these tests tend to touch the most relevant parts of an application at a UI level, I noticed the tests are barely mentioned in the README (perhaps I just missed it and I duplicated some work), so I tried to setup my dev env to run them but surprisingly there was no place other than the pipeline and a package.json where they get called so no instructions again.

This PR aims to start maybe a broader conversation on the automated test strategy but I would like to start small so other can also join and collaborate. For what is related to the changes, I'm adding some instructions to the README about the cypress tests (or what I tend to call UI tests regardless the type of E2e or integration) and a small change on the docker-compose.yml file where a new DB service for the tests was added. This last bit of the docker compose change can be omitted but then we need to include some instructions specifically for the tests (there are some already but for my taste is not too informative)

Jira link:

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Passes test case
• UI components visually reviewed on desktop view
• UI components visually reviewed on mobile view
• Other (please explain)

Checklist:

• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• The precommit and linting ran successfully
• I have added or updated language tags for text that's part of the UI
• I have added "MISSING" for all new language tags to languages I don't speak
• I have added the GNU General Public License to all new files

[atrevino89]

please point out any wording / grammar issue and suggestions to fix them if the PR gains some attention :)

[kathyavini]

Wow, thanks so much for bringing this up @atrevino89! I think this might be the first time that someone has taken interest in the end-to-end tests as part of the contributor workflow!

I love the idea of a docker container for a testing db -- I've definitely forgotten to reconfigure my .env variables and have thrown some junk Cypress data into my main db before. I left some comments below on the docker-compose file, since it was throwing errors for me. I'm assuming you're running on Linux (from the appended :Z's!) so maybe it's a difference between MacOS and Linux there?

As for what you wrote here:

I wanted to execute the E2e tests because for me its easier to take a tour on the app like that as these tests tend to touch the most relevant parts of an application at a UI level

I'm not sure I would say our end-to-end tests really achieve that. They do test some of the most basic elements of the app (if helpful there is a summary document I made for product about their scope), and we run them in CI to safeguard against really egregious regressions only. But they are pretty flakey, and they aren't really part of our developer workflow at all.

In fact, I could only get 1 out of the 3 to ever pass locally while I was testing your branch with that headless run command, and I'm curious if you've had more success, or found them to be useful to you while you were making the PR. We do mostly ignore them, to be honest.

[kathyavini]

I would probably push spinning up a container a little more strongly, because for sure you can run Cypress as long as you have a dev db set up, but that's gonna pollute the DB with Cypress testing data and that's really no good! Maybe instead of 'you'll need to have a DB configured' maybe just go right to:

"When running Cypress E2E tests, it's recommended to use a dedicated test database. This helps keep your main development database free from testing data."

(And similarly maybe remove 'Only if not already configured' because again having set up the main dev DB might feel like enough. I would just go right to

"Start the dedicated test database container with docker compose up cypress-db")

[kathyavini]

I'm not sure why your numbers are all "1" 😂

Also the migration scripts all live in packages/api not in packages/end-to-end

[atrevino89]

the "1's" is one of the best tricks Markdown has 😂 this way you avoid editing the numbers whenever there's a change in the order and they will render in the browser properly, for instance .

[kathyavini]

My shell didn't prompt me! Did it prompt you?

[atrevino89]

Yeah it does ! maybe you need to scroll a bit.
I checked the CI and it has the same
This screenshot is from my shell:

[kathyavini]

Curious -- why the persistent volume? To avoid running the migration again? Could the migration be added to the setup for the test db container instead, and the data purged when bringing down the container? I think a testing db is the perfect case for for wanting start fresh each time the container runs, e.g. with tmpfs.

Unfortunately I think at least some of the end-to-end tests don't work well being run repeatedly -- I think there are some issues with spotlights, which have to be clicked through on first login but aren't shown subsequently. When I had to work on end-to-end locally I actually needed a script that dropped and re-migrated between each run.

[atrevino89]

You are totally right, we don't need a volume here as the whole idea is to spin up a fresh DB with whatever is necessary to run the tests, my bad :)

I need to check about running the migration at the setup to me it sounds that we have 3 options:

1. Setting up an image that contains PostgreSql and the backend (to run the migrations) with all its dependencies and use it in the existing docker-compose.yaml
2. Create a separate docker-compose.yaml with 2 services... 1 for the cypress-db and 1 for the backend so we create the DB first, execute the init script.sql and when it's done start the backend service that will run the migration.
3. do the same as above using the existing docker compose under a testing profile (but it could look a bit more messy)

If I'm over complicating let me know so we can figure out a simpler solution.

About the E2e issues, I will try to pay close attention to them, if anything comes up I let you know hopefully with a fix.

[kathyavini]

I'm getting an error about the options here that prevent me from running this file:

this syntax works for me though:

healthcheck:
  test: ["CMD", "pg_isready", "-U", "postgres"]
  interval: 5s
  timeout: 5s
  retries: 5

[atrevino89]

That's strange, I used your suggestion and both worked for me, although Im leaving yours so it's compatible with anyone else :)

[kathyavini]

Could you put this on a profile (e.g. testing) so it doesn't start up with docker compose up? We want to keep docker compose up as the default, no-config way to get started and it will now error on the port if both databases are included

Bind for 0.0.0.0:5433 failed: port is already allocated

[atrevino89]

fixed !

[kathyavini]

Should it be called something like testing-db instead? I see you included the creation of test_farm which is not used by Cypress under the default config (it's just for the API tests). I thought maybe from the readme changes as well that this container could be used for both?

[atrevino89]

fixed

[atrevino89]

Hi @kathyavini thanks a lot for such enriching feedback!

I'll try to address everything you mentioned and maybe start a bunch of new tasks based on the automation testing subject.

I'm assuming you're running on Linux (from the appended :Z's!) so maybe it's a difference between MacOS and Linux there

According to what I read, this shouldn't affect to MacOs users, docker ignores the :z label as it doesn't use that feature on MacOs, it should know this is a SELinux property. Anyway I'll remove it to avoid any further issue.

[...] we run them in CI to safeguard against really egregious regressions only. But they are pretty flakey, and they aren't really part of our developer workflow at all.

Having the E2e tests and running them in a CI is a big step to the right direction :)
About not being part of the developer workflow I think that's fine nevertheless they could be a bit more efficient so someone can use them to support any change and make sure it doesn't break anything... also there's alway the option of breaking the e2e tests down in different test layers which in turn it will make all of tests less flaky, more isolated and more reliable. I see you guys having some tests already, just a matter of checking how much of E2e is really needed and what other areas can get more coverage, if there's anythng relevant to my eyes, I'll try to make suggestions in a near future.

In fact, I could only get 1 out of the 3 to ever pass locally while I was testing your branch with that headless run command, and I'm curious if you've had more success, or found them to be useful to you while you were making the PR. We do mostly ignore them, to be honest.

No worries, not even without my changes I could make it work at least once... I think some work has to be done around them but I'll check in the next days... I'll address that on a separate PR and creating a jira ticket so you guys are aware that I'm tweaking them.