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(#9443): offline user e2e test coverage for tasks #9498

Merged
merged 26 commits into from
Oct 23, 2024

Conversation

ralfudx
Copy link
Contributor

@ralfudx ralfudx commented Oct 3, 2024

Description

#9443

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.

@ralfudx ralfudx self-assigned this Oct 3, 2024
@ralfudx ralfudx marked this pull request as draft October 3, 2024 22:29
@ralfudx ralfudx marked this pull request as ready for review October 14, 2024 09:08
@ralfudx
Copy link
Contributor Author

ralfudx commented Oct 14, 2024

@tatilepizs @lorerod @kapelo kindly have a look when you have some time

@ralfudx
Copy link
Contributor Author

ralfudx commented Oct 15, 2024

@tatilepizs @lorerod @kapelo gentle reminder to kindly have a look at this

@tatilepizs
Copy link
Contributor

Thank you for the heads up @ralfudx, I have been busy with other things that I have in my queue; I will review it as soon as I can, no later than the end of the week.

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 for these tests @ralfudx.
I left some suggestions and questions 😊

tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/page-objects/default/tasks/tasks.wdio.page.js Outdated Show resolved Hide resolved
tests/page-objects/default/tasks/tasks.wdio.page.js Outdated Show resolved Hide resolved
@ralfudx
Copy link
Contributor Author

ralfudx commented Oct 17, 2024

@tatilepizs I have made the requested changes and also added comments to the questions asked
kindly have a second look

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 for the updates @ralfudx.
I left some comments and some follow-ups to previous comments. I think that you missed two comments, this and this 🙂

tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
@ralfudx
Copy link
Contributor Author

ralfudx commented Oct 17, 2024

Thanks for reviewing this @tatilepizs I have checked the new comments and follow-ups and made the requested modifications - Kindly have a look again

@ralfudx
Copy link
Contributor Author

ralfudx commented Oct 18, 2024

Gentle reminder on this @tatilepizs @lorerod - Kindly review

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.

Thanks Rafa
I left some comments

tests/page-objects/default/tasks/tasks.wdio.page.js Outdated Show resolved Hide resolved
tests/page-objects/default/tasks/tasks.wdio.page.js Outdated Show resolved Hide resolved
tests/page-objects/default/tasks/tasks.wdio.page.js Outdated Show resolved Hide resolved
@ralfudx
Copy link
Contributor Author

ralfudx commented Oct 21, 2024

Thanks for the feedback @tatilepizs i have made the requested changes

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 for being so patient and so open to suggestions ⭐

The tasks tests are failing due to compileTasks because not for all the tests we can sync, there are some of them where the user is not logged in when we update the settings, so we can't sync at that moment.
I left suggestions about how to fix them

But everything else looks great 🚀

tests/page-objects/default/tasks/tasks.wdio.page.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks-breadcrumbs.wdio-spec.js Outdated Show resolved Hide resolved
tests/e2e/default/tasks/tasks-group.wdio-spec.js Outdated Show resolved Hide resolved
@ralfudx ralfudx linked an issue Oct 22, 2024 that may be closed by this pull request
3 tasks
@ralfudx ralfudx merged commit fab7764 into master Oct 23, 2024
44 checks passed
@ralfudx ralfudx deleted the 9443-offline-user-e2e-test-coverage-for-tasks branch October 23, 2024 12:33
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.

Add e2e test coverage to tasks for offline user
2 participants