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: backporting e2e fix #8688

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Conversation

latin-panda
Copy link
Contributor

@latin-panda latin-panda commented Nov 8, 2023

Description

Backporting 956f81a

Code review checklist

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or e2e where appropriate
  • Internationalised: All user facing text
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

Compose URLs

If Build CI hasn't passed, these may 404:

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

@lorerod can you please check this small PR? It's a backport of a fix that will allow the unblocking of the CI for this Feature Release. Thanks for your time!

@latin-panda latin-panda mentioned this pull request Nov 8, 2023
5 tasks
Copy link
Contributor

@lorerod lorerod left a comment

Choose a reason for hiding this comment

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

Thank to you Jenni.
I left just a little question.

await commonPage.goToReports();

await browser.url('#/reports/add/pregnancy');
await browser.url('/#/reports/add/pregnancy');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we don't use here the new commonPage.goToUrl() method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a very good question. I'm not the original author, but it seems like commonPage.goToUrl() should work here.
Do you think we should encourage the usage of commonPage.goToUrl() always as a replacement for browser.url(..?
If yes, I can open a separate PR to do the replacement all over the e2e, so that's the way to go for other developers when writing tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is a great idea to do this.

@latin-panda latin-panda merged commit 54cb3cf into 4.4.1-FR-barcode Nov 9, 2023
31 checks passed
@latin-panda latin-panda deleted the backporting-e2e-fix branch November 9, 2023 02:27
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.

3 participants