Skip to content

chore: complete apdex automation tasks#9247

Merged
ralfudx merged 26 commits intoapdex-automation-testsfrom
complete-apdex-automation-tasks
Jul 22, 2024
Merged

chore: complete apdex automation tasks#9247
ralfudx merged 26 commits intoapdex-automation-testsfrom
complete-apdex-automation-tasks

Conversation

@ralfudx
Copy link
Copy Markdown
Contributor

@ralfudx ralfudx commented Jul 4, 2024

Description

Merge test scenario to capture telemetry data for tasks:refresh into apdex-automation-tests branch and clean up code

#142

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
Copy link
Copy Markdown
Contributor Author

ralfudx commented Jul 10, 2024

Hey the build finally passed thanks to Diana's contribution - @latin-panda @tatilepizs kindly have a look when you have some time

Copy link
Copy Markdown
Contributor

@latin-panda latin-panda left a comment

Choose a reason for hiding this comment

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

Looking good
Please do another check to remove all unused code and unused files, just to leave this nice and tidy

Comment thread tests/performance/apdex-score/test/page-objects/contacts.page.js Outdated
Comment thread tests/performance/apdex-score/test/page-objects/page.js Outdated
Copy link
Copy Markdown
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 Rafa for updating the tests ⭐

I left a couple of suggestions, and have one question.

Maybe I am a little late on this, but is there a specific reason why you decided to add a page-objects folder inside the apdex-score/test folder instead of adding a apdex folder inside the page-objects folders that we already have?

I think it will be better to have all the page objects files in one place as we are doing with all the other tests suites.

Comment thread tests/performance/apdex-score/test/page-objects/load.page.js Outdated
Comment thread tests/performance/apdex-score/test/page-objects/login.page.js Outdated
Comment thread tests/performance/apdex-score/test/page-objects/login.page.js Outdated
@ralfudx
Copy link
Copy Markdown
Contributor Author

ralfudx commented Jul 15, 2024

Thanks for the comments and suggestions @tatilepizs I'll have a look and see if the recommendations can fit in
Also, I guess we can have a quick chat over this during one of the QA sync calls to better align on some of these suggestions

@ralfudx
Copy link
Copy Markdown
Contributor Author

ralfudx commented Jul 15, 2024

@tatilepizs In order not to delay the apdex tasks completion work I'd suggest we include these structural changes in a different/subsequent PR
This will allow us to quickly merge this PR to the apdex-automation-tests branch (as part of this week's commitment for care team) so that @latin-panda is unblocked with her additional changes for the settings file

@tatilepizs
Copy link
Copy Markdown
Contributor

Ok, that would be fine. but can you please create a ticket to address those changes? if we don't do it we will probably will forget and those e2e tests will start growing and then it will be harder to make those changes.

@ralfudx
Copy link
Copy Markdown
Contributor Author

ralfudx commented Jul 16, 2024

Sure this has been created to track

@latin-panda
Copy link
Copy Markdown
Contributor

latin-panda commented Jul 16, 2024

I'd suggest we include these structural changes in a different/subsequent PR

@ralfudx Do you mean this feedback ⬇️ from Tatiana?

Maybe I am a little late on this, but is there a specific reason why you decided to add a page-objects folder inside the apdex-score/test folder instead of adding a apdex folder inside the page-objects folders that we already have?

If it's just moving files around to another folder and updating imports, let's do it! Because it's not a big coding change.

@tatilepizs @ralfudx do you mean something like this? ⬇️

|-tests/
|---performance/
|------apdex-score/
|---------page-objects/
|------------page.js
|---------tests.js <--- a file containing the test cases (currently apdex-score/test/specs/num-1/test.apdex.js)

@ralfudx please remember to remove all unused files and code. For example we don't need anymore these files because we use setting files to test different deployments:
Screenshot 2024-07-16 at 1 14 54 PM

That leaves us with 1 file only with test cases: apdex-score/test/specs/num-1/test.apdex.js. So, simplifying the folders makes sense because we only have 1 file, so following the suggestion from restructuring, it's something like this: apdex-score/tests.js.

@ralfudx
Copy link
Copy Markdown
Contributor Author

ralfudx commented Jul 16, 2024

@latin-panda I have removed the unused test files

I believe this is the structure @tatilepizs is referring to:
|-tests/
|---page-objects/
|------apdex <---- a folder containing all the apdex page.js files (moved from apdex-score/test/page-objects)
|---performance/
|------apdex-score/
|---------specs
|------------test.apdex.js <--- a file containing the test cases (previously apdex-score/test/specs/num-1/test.apdex.js)

Other suggestions include code changes related to selector definition, using a method to capture selectors and also changes to the wdio.conf.js file similar to what was done in cht-form

If we intend to do all these changes in this PR I cannot commit to completing this work Tuesday EOD as mentioned in Care Team's Chat
Also, I believe these changes are out-of-scope for the work intended here and should/can be captured in the larger apdex-automation-tests Draft PR

We also need to keep in mind that in the future, we may use this project for other types of mobile testing apart from apdex

@latin-panda
Copy link
Copy Markdown
Contributor

I believe this is the structure @tatilepizs is referring to:
|-tests/
|---page-objects/
|------apdex <---- a folder containing all the apdex page.js files (moved from apdex-score/test/page-objects)
|---performance/
|------apdex-score/
|---------specs
|------------test.apdex.js <--- a file containing the test cases (previously apdex-score/test/specs/num-1/test.apdex.js)

@ralfudx, that structure still makes sense to me 👍

I sent you a snippet of the button code here, small & safe change. I didn't have time to help and test the XPATH selector's feedback.

If we intend to do all these changes in this PR I cannot commit to completing this work Tuesday EOD as mentioned in Care Team's Chat
Also, I believe these changes are out-of-scope for the work intended here and should/can be captured in the larger #9037

It seems Michael is requesting the entire Apdex work to be merged directly into cht-core by EOD Tuesday, as mentioned in the Care Teams' summary. The intention is to effectively complete this feature that has been open since April . Please don't compromise code and solution quality, and do not let tickets be done later because this is still a manageable size of project and historically we don't retake those tickets in years.
To summarize, it's fine to do some of these fixes in the larger PR, but not leave them for later.

@ralfudx
Copy link
Copy Markdown
Contributor Author

ralfudx commented Jul 16, 2024

@latin-panda @tatilepizs kindly re-review
Thanks

Copy link
Copy Markdown
Contributor

@latin-panda latin-panda left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning all that code! 🤩
I just did a quick pass because I'm running out of time, I won't be able to review for the rest of the week, so I'm unblocking this PR and let @tatilepizs continue with the review.

Comment thread tests/page-objects/apdex/page.js Outdated
Comment thread tests/page-objects/apdex/page.js Outdated
Comment thread tests/performance/apdex-score/README.md Outdated
@latin-panda latin-panda dismissed their stale review July 16, 2024 11:51

I won't be able to review this week. Unblocking this PR. Letting @tatilepizs to continue with the review.

ralfudx and others added 3 commits July 16, 2024 14:57
Co-authored-by: Jennifer Q <66472237+latin-panda@users.noreply.github.com>
Copy link
Copy Markdown
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 all the changes @ralfudx ! ⭐
I left some small changes

Comment thread tests/page-objects/apdex/load.page.js Outdated
Comment thread tests/page-objects/apdex/login.page.js Outdated
Comment thread tests/page-objects/apdex/tasks.page.js Outdated
Comment thread tests/performance/apdex-score/specs/test.apdex.js Outdated
Comment thread tests/page-objects/apdex/load.page.js Outdated
@ralfudx
Copy link
Copy Markdown
Contributor Author

ralfudx commented Jul 19, 2024

@tatilepizs all requested changes have now been resolved - kindly have take out some time to review again on Monday thanks for all the feedback

@ralfudx ralfudx linked an issue Jul 19, 2024 that may be closed by this pull request
3 tasks
Copy link
Copy Markdown
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 looks good @ralfudx, thank you for all the changes 🌟

@ralfudx ralfudx merged commit 926c366 into apdex-automation-tests Jul 22, 2024
@ralfudx ralfudx deleted the complete-apdex-automation-tasks branch July 22, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants