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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add steps to run upgrade e2e #1218

Merged
merged 13 commits into from
Dec 4, 2023

Conversation

latin-panda
Copy link
Contributor

Description

  • Adds steps to run upgrade e2e with some troubleshooting steps.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@latin-panda
Copy link
Contributor Author

Hi @lorerod and @tatilepizs can any of you please try these steps and provide feedback? :) Thanks!

- A way to do this is by pushing the branch, let the GitHubActions to run, if all the other e2e as okay, then it will publish the branch.
- Check that your branch name is available [here](https://staging.dev.medicmobile.org/_couch/builds_4/_design/builds/_view/releases).
- Make sure to stop all existing containers
- Run CHT as usual:
Copy link
Contributor

Choose a reason for hiding this comment

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

What about couchdb? with what config? I used this.
Maybe it is better to reference dev environment setup

```
Try the following:
- Make sure to stop all existing containers, because maybe the Nginx port was already allocated and it couldn't start.
- If you keep getting errors, check that the Nginx ports are available.
Copy link
Contributor

Choose a reason for hiding this comment

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

I keep getting the error:
Error in hook: StatusCodeError: 404 - "{\"error\":\"not_found\",\"reason\":\"Document is missing attachment\"}\n"
What did you do to fix it?

Copy link
Contributor Author

@latin-panda latin-panda Nov 3, 2023

Choose a reason for hiding this comment

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

I ran wdio-local before and that fixed it but I don't think that's the proper way.
@tatilepizs is also getting the same error

@dianabarsan, what other troubleshooting steps can they follow here?

Copy link
Contributor

@tatilepizs tatilepizs left a comment

Choose a reason for hiding this comment

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

It is not working for me @latin-panda.

First I tried following the exact same steps that you add, and to able to have a CHT instance up and running I needed a container running. I saw that error that you point, so I stoped the container and tried to run the command without an cht instance running, but I am still seeing the same error.

Images with cht instance up and running, one container

image
image

Images without a cht instance.

image

content/en/contribute/code/core/fixing-e2e-tests.md Outdated Show resolved Hide resolved
- Make sure your branch has been published and it's available in the market:
- A way to do this is by pushing the branch, let the GitHubActions to run, if all the other e2e as okay, then it will publish the branch.
- Check that your branch name is available [here](https://staging.dev.medicmobile.org/_couch/builds_4/_design/builds/_view/releases).
- Make sure to stop all existing containers
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean with this step? Because you need the cht-docker container to be able to run cht-core.
Maybe you could add the link to the CouchDB Setup and say that only that container should be running.

Comment on lines 43 to 46
- Run CHT as usual:
- `npm run build-dev-watch`
- `npm run dev-api`
- `npm run dev-sentinel`
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add the link to run the CHT? It will be easier in case someone needs more help

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this steps are not needed. npm run upgrade-wdio should set up everything.

- `export MARKET_URL_READ=https://staging.dev.medicmobile.org`
- `export STAGING_SERVER=_couch/builds`
- `export BRANCH=<your branch name>`
- Run the upgrade e2e tests: `npm run upgrade-wdio`
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add where should that command be run? I assumed it was in the cht-core/ root level, but it would be a good idea to confirm it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It took some debugging but the correct STAGIG_SERVER for 4.x releases is _couch/builds_4
While for anything previous 4.x the value should be _couch/builds

@tatilepizs
Copy link
Contributor

Hi @latin-panda, @dianabarsan, @garethbowen,

Do you have any update about how can we run the upgrade e2e tests?
@lorerod has been trying to fix the upgrade.wdio-spec.js test and I am trying to help, but it is really hard to do it if we cannot run that test locally, we are going blind pushing commits in the CI to see if we have luck 😅

Could someone please help us with steps that we can follow to run the upgrade tests locally?

@garethbowen
Copy link
Member

@tatilepizs Sorry I'm flat out right now. @dianabarsan is the expert on this test and she's back tomorrow so should be able to help then.

@latin-panda
Copy link
Contributor Author

@tatilepizs @garethbowen
I had a work session with @lorerod and we both figured out a way to run these tests in the local env. It's not the proper way but it works, and enough for now to unblock. I'm going to update the docs shortly, and we can improve them in a second iteration.

…dding_steps_to_run_upgrade_e2e

# Conflicts:
#	content/en/contribute/code/core/fixing-e2e-tests.md
- To upgrade from previous versions of CHT use: `export STAGING_SERVER=_couch/builds`.
- `export BRANCH=<your branch name>`.
- Start the containers for e2e tests like this
- TBD
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To start the container, for now, we need to add this:
Screenshot 2023-11-14 at 10 44 54 pm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lorerod FYI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dianabarsan pinging you on this again.
These changes allow the upgrade tests in the local environment to run, but they still fail while in the master's CI the pass. This probably isn't the correct way to run them.

[0-0] AssertionError in "Performing an upgrade.should have an upgrade_log after installing"
AssertionError: expected +0 to equal 1
    at Context.<anonymous> (/Users/tlepiz/Documents/Medic/Repos/PRs/cht-core/tests/e2e/upgrade/upgrade.wdio-spec.js:82:28)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
[0-0] Error in "Performing an upgrade.should upgrade to current branch"
Error: element ("div*=Deployment complete") still not displayed after 15000ms
    at async Context.<anonymous> (/Users/tlepiz/Documents/Medic/Repos/PRs/cht-core/tests/e2e/upgrade/upgrade.wdio-spec.js:103:5)

Video from Tatiana

@tatilepizs @lorerod and I are thinking that we're missing some setup. The QA Team wants to learn and document how to run these tests in their local environment to debug whenever needed.

What are we missing here? What is the best way to run them in the local environment?

Copy link
Member

Choose a reason for hiding this comment

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

I went over the upgrade e2e test locally and everything worked. I made no changes to the upgrade e2e test, as suggested above, and those changes should not be necessary.

My experience was:

  • I needed to stop existing containers that cause port collisions
  • on slow internet, the tests failed because the images weren't downloaded in sufficient time, this just meant I had to restart the test a bunch of times until the images were downloaded
  • I had to checkout and run the tests over the branch I wanted to upgrade to. This is mainly necessary to compute the correct version within the test.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

@dianabarsan can you please share the exact steps that you are doing to run the tests?
I still need to run npm run wdio-local to have all the containers ready and then run npm run upgrade-wdio and it still fails 😞

You pointed out that maybe on slow internet it will fail, how fast my internet needs to be, maybe my internet is too slow 😅. Is there a way to check if the images started the download process but they didn't finish? Because I ran the test 10 times and it is still failing. How many times do you think that it would require to be run to download all the images successfully?

Screen.Recording.2023-11-17.at.11.32.17.am.mov

Copy link
Member

@dianabarsan dianabarsan Nov 23, 2023

Choose a reason for hiding this comment

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

Can you change that path locally here: https://github.com/medic/cht-core/blob/b8519be2e415deaf20137c80ce55d9257321cff3/tests/e2e/upgrade/wdio.conf.js#L64 and try again?

DOCKER_CONFIG_PATH: os.homedir(),

Copy link
Contributor

Choose a reason for hiding this comment

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

Diana!!! 🤯

image

I just changed it to an empty string (because that is the value of $homeDir) -> DOCKER_CONFIG_PATH: '',

Copy link
Member

Choose a reason for hiding this comment

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

Cool! So happy we got to the bottom of this! Thanks for being patient.
I believe we should just change the code to take this value from an environment variable. I'll try to push a change shortly, and ask you to try the upgrade again (in a few hours) and then we can document the new environment variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect!
Thank you so much for all the help!! 🌟

Copy link
Contributor

Choose a reason for hiding this comment

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

@tatilepizs @dianabarsan it works!! Thank you:
For me it worked changing to DOCKER_CONFIG_PATH: os.homedir(),
I tested manually downloaded the images and without the images.

- `export MARKET_URL_READ=https://staging.dev.medicmobile.org`.
- `export STAGING_SERVER=<builds path>`.
- To upgrade from CHT v4.x.x use `export STAGING_SERVER=_couch/builds_4`.
- To upgrade from previous versions of CHT use: `export STAGING_SERVER=_couch/builds`.
Copy link
Member

@dianabarsan dianabarsan Nov 17, 2023

Choose a reason for hiding this comment

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

I don't believe you can upgrade from previous 3.x versions, because those do not have docker compose files. We should remove this line.

- To upgrade from previous versions of CHT use: `export STAGING_SERVER=_couch/builds`.
- `export BRANCH=<your branch name>`.
- Start the containers for e2e tests like this
- TBD
Copy link
Member

Choose a reason for hiding this comment

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

I went over the upgrade e2e test locally and everything worked. I made no changes to the upgrade e2e test, as suggested above, and those changes should not be necessary.

My experience was:

  • I needed to stop existing containers that cause port collisions
  • on slow internet, the tests failed because the images weren't downloaded in sufficient time, this just meant I had to restart the test a bunch of times until the images were downloaded
  • I had to checkout and run the tests over the branch I wanted to upgrade to. This is mainly necessary to compute the correct version within the test.

image

@lorerod
Copy link
Contributor

lorerod commented Dec 4, 2023

@tatilepizs @dianabarsan can you please re-review this PR? Since the issue was fixed in chore(#8727): change docker socket path in upgrade e2e test.

Copy link
Member

@dianabarsan dianabarsan left a comment

Choose a reason for hiding this comment

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

This is great!

Copy link
Contributor

@tatilepizs tatilepizs left a comment

Choose a reason for hiding this comment

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

Thank you @lorerod for updating the steps.

@lorerod lorerod self-requested a review December 4, 2023 19:30
@lorerod lorerod merged commit 70f527f into main Dec 4, 2023
2 checks passed
@lorerod lorerod deleted the chore_adding_steps_to_run_upgrade_e2e branch December 4, 2023 20:31
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.

5 participants