chore(#109): Run test suite from the apdex test automation branch #9114

ralfudx · 2024-05-13T21:18:41Z

Description

#109

To review this you can do the following:

check the files changed and ensure there's nothing strange/out of place
Run npm ci from the repository's root directory
Connect your mobile device and then run npm run apdex-test
The tests should start running on the physical device without any error.

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 · 2024-05-14T10:08:37Z

I have asked for help on the dev channel for the failing CI builds

latin-panda

@ralfudx, the CI it's throwing an error that says:

npm ERR! npm ci can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync.

In this PR, you have made changes in package.json only, so it's not in sync with the package-lock.json.

You need to run:
npm install @wdio/appium-service --save-dev
npm install appium-uiautomator2-driver --save-dev
npm install ts-node --save-dev
Then, commit and push those changes of package-lock.json. It should only contain changes from those 3 new dependencies.
Once you have that, then remove the following from apdex-score/package.json because we already have those dependencies in the main package.json of CHT:

@wdio/allure-reporter
@wdio/appium-service
@wdio/cli
@wdio/local-runner
@wdio/mocha-framework
@wdio/spec-reporter
allure-commandline
faker we already have `@faker-js/faker`
ts-node
typescript

The apdex-score folder should also have a package-lock.js file. To get it properly generated, install each dependencies manually.
I have these questions:

Do we really really need "moment-timezone"?
Is it possible to keep "appium-uiautomator2-driver" in apdex-score/package.json ?

ralfudx · 2024-05-15T08:47:42Z

npm install @wdio/appium-service --save-dev
npm install appium-uiautomator2-driver --save-dev
npm install ts-node --save-dev

@latin-panda thanks for the comment and requested changes - I had previously done these steps and I just did them again however, the build is still failing with the error I reported in the dev channel and I'm not sure why this is still happening but I noticed that even when i install the dependencies one by one with the commands you suggested above it seems the package-lock.json file doesn't contain only changes from the 3 new dependencies because I can see the file having 34,317 additions, 23,184 deletions.
At this point, I must say I'm out of ideas and will appreciate any help on this

To answer your questions:

We don't need moment-timezone and I've removed it.
If we keep appium-uiautomator2-driver in apdex-score/package.json the purpose of running the tests directly from the cht-core root directory will be defeated.

I have these questions though:

Is there a specific reason why you need/want us to keep appium-uiautomator2-driver in apdex-score/package.json
Since we are managing dependencies and running everything from the main package.json of CHT, do we really need to still keep the apdex-score/package.json file?

latin-panda · 2024-05-15T10:46:17Z

Is there a specific reason why you need/want us to keep appium-uiautomator2-driver in apdex-score/package.jsonIs there a specific reason why you need/want us to keep appium-uiautomator2-driver in apdex-score/package.json

Maybe it was nice to keep it in apdex-score/package.json because it's only used there. But if it's not possible, then it's okay in the main package.json

Since we are managing dependencies and running everything from the main package.json of CHT, do we really need to still keep the apdex-score/package.json file?Since we are managing dependencies and running everything from the main package.json of CHT, do we really need to still keep the apdex-score/package.json file?

Good question. If the file ends up without dependencies to install and no scripts defined, I think it makes sense to remove it. 🤔

ralfudx · 2024-05-15T11:00:10Z

@latin-panda @lorerod @Benmuiruri can you kindly help review this? I've only made changes to the package.json file so it should be quick i guess? 😃

latin-panda

@ralfudx I'm getting this error - I tried with sudo and still not working

The apk is in the usual place

Are you getting the same error?

Also if we don't need the scripts in the apdex-score/package.json, I think you can delete the file in this PR.

 "scripts": {
    "test": "npx wdio wdio.conf.js",
    "report": "npx allure generate allure-results --clean && npx allure open",
    "apdex-test": "wdio run ./wdio.conf.js"
  },

ralfudx · 2024-05-16T10:34:52Z

Yes @latin-panda this is because the path to the apk in the wdio.conf.js file was not properly updated - You can even comment that line out if you already have the app installed on the physical device.
I have updated the path to the apk and removed the unused package files and it's running fine now. Please go ahead and try it again

ralfudx · 2024-05-16T20:01:10Z

@Benmuiruri and @latin-panda I'd appreciate a final review on this so it can be merged this week

latin-panda · 2024-05-17T05:22:33Z

@ralfudx I pulled the latest from the branch, but it is still not working after a fresh install of the repo. It seems some files are using that moment-timezone, and the browser isn't properly installed - maybe it is a missing dependency or maybe because the dependencies were downgraded?

How are you running these tests in your local environment?

ralfudx · 2024-05-17T06:16:00Z

@latin-panda I pulled the latest from apdex-automation-tests branch before making these changes and I'm using the npm run apdex-test command to run the tests from the root directory of cht-core

latin-panda · 2024-05-17T06:21:38Z

@ralfudx if you remove the node_modules from the root folder and apdex-score folder, and then npm ci, do you get the same error?

ralfudx · 2024-05-17T08:34:10Z

@latin-panda yes I get the same error after doing npm ci and it appears it's missing a dependency
I added moment-timezone and that fixed the issue. You can give it another try

latin-panda · 2024-05-17T10:31:09Z

@ralfudx, but here, you mentioned that we don't need moment-timezone

We don't need moment-timezone and I've removed it.

It is better to remove or comment the import of this dependency in those files instead of installing the dependency into CHT. We try to add just the necessary to CHT to keep it slim

ralfudx · 2024-05-17T14:06:45Z

@latin-panda after having a deeper look I noticed we are still using it in test/page-objects/page.js to define and extract the current date+time in order to calculate the lmpDate, followUpDate, & vhtVisitDate and that's why i added it

latin-panda · 2024-05-20T04:56:57Z

I see thanks @ralfudx; I took a deeper look this morning. Those values are for filling out the forms, right? They will need to be included in the form's configuration of the settings file before running the test suite every time. So, no dynamic dates are required.

However, if we still need date conversion in an edge case, the existing e2e tests use the regular moment library to calculate the fields. That library is already available in CHT.

Therefore, it is better not to add moment-timezone now. Let's leave it as the last resource in case of an edge case.
Please remove the moment-timezone and comment or remove the code using it.

ralfudx · 2024-05-20T16:22:26Z

@latin-panda i have removed moment-timezone and commented the code using it

latin-panda

@ralfudx I've tested the branch and it's running the test properly with the settings file (see video)

Screen.Recording.2024-05-21.at.12.47.16.PM.mp4

However the moment-timezone wasn't properly uninstalled and the package-lock.json still has the dependency defined. (See that package-lock has 57k lines, it should be less than 40k)

Run npm uninstall moment-timezone in the root folder, that should update the package-lock

ralfudx · 2024-05-21T06:42:14Z

@latin-panda thanks for the observation, I have done this and even after running npm uninstall moment-timezone in the root folder, it seems the dependency is still listed in the package-lock.js file
I can still see 57k lines in that file - is this something that needs to be resolved manually?

latin-panda

@ralfudx It's better to not fix the package-lock file because it has some special logic that is autogenerated and that can cause unexpected behaviors in the apps.

When the issue you described happens, it's better to install it (npm install moment-timezone --save-dev) and then uninstall it (npm uninstall moment-timezone --save-dev), npm will handle the package-lock and void problems.

ralfudx · 2024-05-21T08:48:33Z

@Benmuiruri and @lorerod if you'd like to provide a quick review as well, I'd appreciate that before we merge this to the apdex-automation-tests branch

Benmuiruri

Hi @ralfudx could you update the PR description with the steps for reviewing it ?

ralfudx · 2024-05-21T14:02:35Z

@Benmuiruri I have updated the description. Kindly let me know if something there is not clear
Thanks

Benmuiruri

LGTM! After making adjustments to Jenni's sample script I was able to run the tests successfully

lorerod

Just a minor suggestion.
Thanks @ralfudx

lorerod · 2024-05-21T20:26:27Z

tests/performance/apdex-score/test/page-objects/page.js

+  // async extractCurrentDate(days) {
+  //   const dateTimeString = await driver.getDeviceTime();
+  //   let dateTime = moment(dateTimeString);
+  //   dateTime = moment(dateTime).add(days, 'days');
+
+  //   const year = dateTime.format('YY');
+  //   const month = dateTime.format('MM');
+  //   const day = dateTime.format('DD');
+  //   const hour = dateTime.format('HH');
+  //   const minute = dateTime.format('mm');
+
+  //   return {year, month, day, hour, minute};
+  // }
+
+  // async updateCurrentDate (days) {
+  //   const extractCurrentDate = await this.extractCurrentDate(days);
+  //   console.log('TIME::: Year:', extractCurrentDate.year);
+  //   console.log('TIME::: Month:', extractCurrentDate.month);
+  //   console.log('TIME::: Day:', extractCurrentDate.day);
+  //   console.log('TIME::: Hour:', extractCurrentDate.hour);
+  //   console.log('TIME::: Minute:', extractCurrentDate.minute);
+  //   console.log('TIME::: Extracted Components:', extractCurrentDate);
+  //   const adbDateFormat = `${extractCurrentDate.month}${extractCurrentDate.day}${extractCurrentDate.hour}
+  //   ${extractCurrentDate.minute}${extractCurrentDate.year}`;
+  //   execSync('adb shell su root date ' + adbDateFormat, { stdio: 'inherit' });
+  //   browser.pause(10000);
+  // }
+
+  // async getLmpDate () {
+  //   const extractLmpDate = await this.extractCurrentDate(-62);
+  //   const lmpDate = `20${extractLmpDate.year}-${extractLmpDate.month}-${extractLmpDate.day}`;
+  //   return lmpDate;
+  // }
+
+  // async getFollowUpDate () {
+  //   const extractNextDate = await this.extractCurrentDate(1);
+  //   const followUpDate = `20${extractNextDate.year}-${extractNextDate.month}-${extractNextDate.day}`;
+  //   return followUpDate;
+  // }
+
+  // async getVHTVisitDate () {
+  //   const extractPreviousDate = await this.extractCurrentDate(-1);
+  //   const visitDate = `20${extractPreviousDate.year}-${extractPreviousDate.month}-${extractPreviousDate.day}`;
+  //   return visitDate;
+  // }


Can you please delete the commented lines?

run-tests-from-cht-core

2daf313

ralfudx changed the base branch from master to apdex-automation-tests May 13, 2024 21:19

ralfudx mentioned this pull request May 13, 2024

chore(#109): Run test suite from the apdex test automation branch in cht-core #9071

Closed

5 tasks

ralfudx linked an issue May 13, 2024 that may be closed by this pull request

Run a test suite from the apdex test automation branch in cht-core medic/care-teams#109

Open

3 tasks

ralfudx mentioned this pull request May 13, 2024

Run a test suite from the apdex test automation branch in cht-core medic/care-teams#109

Open

3 tasks

ralfudx requested review from Benmuiruri, lorerod and latin-panda May 13, 2024 22:18

updated package.json file

4382d09

ralfudx force-pushed the run-tests-from-cht-core branch from 5eb19da to 4382d09 Compare May 14, 2024 11:19

Rafa added 4 commits May 14, 2024 12:29

restored-package-lock.json

a6aa3fb

changed-version

1c0069c

updated package-lock.json

5e20ad0

reverted-changes

ecb3598

latin-panda requested changes May 15, 2024

View reviewed changes

added-dependencies-one-by-one

5ba6568

Rafa added 2 commits May 15, 2024 10:57

downgraded-appium-uiautomator-version

ca7ea7e

reinstalled-typescript-5.3.3

62b9cc3

ralfudx requested a review from latin-panda May 15, 2024 11:00

latin-panda reviewed May 16, 2024

View reviewed changes

removed-apdex-score-package.json

cbb24a9

added-moment-timezone

38f096b

removed-moment-timezone

f15984b

latin-panda requested changes May 21, 2024

View reviewed changes

removed-moment-timezone2

de6f131

resolved-lock-file

4f9bef3

latin-panda approved these changes May 21, 2024

View reviewed changes

Benmuiruri reviewed May 21, 2024

View reviewed changes

Benmuiruri approved these changes May 21, 2024

View reviewed changes

ralfudx merged commit 3a2f4f7 into apdex-automation-tests May 21, 2024
31 checks passed

ralfudx deleted the run-tests-from-cht-core branch May 21, 2024 21:27

lorerod approved these changes May 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(#109): Run test suite from the apdex test automation branch #9114

chore(#109): Run test suite from the apdex test automation branch #9114

ralfudx commented May 13, 2024 •

edited

ralfudx commented May 14, 2024

latin-panda left a comment

ralfudx commented May 15, 2024

latin-panda commented May 15, 2024

ralfudx commented May 15, 2024

latin-panda left a comment

ralfudx commented May 16, 2024

ralfudx commented May 16, 2024

latin-panda commented May 17, 2024

ralfudx commented May 17, 2024

latin-panda commented May 17, 2024 •

edited

ralfudx commented May 17, 2024

latin-panda commented May 17, 2024

ralfudx commented May 17, 2024

latin-panda commented May 20, 2024

ralfudx commented May 20, 2024

latin-panda left a comment

ralfudx commented May 21, 2024

latin-panda left a comment

ralfudx commented May 21, 2024

Benmuiruri left a comment

ralfudx commented May 21, 2024

Benmuiruri left a comment

lorerod left a comment

lorerod May 21, 2024

chore(#109): Run test suite from the apdex test automation branch #9114

chore(#109): Run test suite from the apdex test automation branch #9114

Conversation

ralfudx commented May 13, 2024 • edited

Description

Code review checklist

Compose URLs

License

ralfudx commented May 14, 2024

latin-panda left a comment

Choose a reason for hiding this comment

ralfudx commented May 15, 2024

latin-panda commented May 15, 2024

ralfudx commented May 15, 2024

latin-panda left a comment

Choose a reason for hiding this comment

ralfudx commented May 16, 2024

ralfudx commented May 16, 2024

latin-panda commented May 17, 2024

ralfudx commented May 17, 2024

latin-panda commented May 17, 2024 • edited

ralfudx commented May 17, 2024

latin-panda commented May 17, 2024

ralfudx commented May 17, 2024

latin-panda commented May 20, 2024

ralfudx commented May 20, 2024

latin-panda left a comment

Choose a reason for hiding this comment

ralfudx commented May 21, 2024

latin-panda left a comment

Choose a reason for hiding this comment

ralfudx commented May 21, 2024

Benmuiruri left a comment

Choose a reason for hiding this comment

ralfudx commented May 21, 2024

Benmuiruri left a comment

Choose a reason for hiding this comment

lorerod left a comment

Choose a reason for hiding this comment

lorerod May 21, 2024

Choose a reason for hiding this comment

ralfudx commented May 13, 2024 •

edited

latin-panda commented May 17, 2024 •

edited