fix(html_report): add if statement

ashrivastava-qa commented 1 month ago

Because

The reports were not generating for parallel machines that had failed tests runs.

This pull request

Added the when parameter to config.yaml file for the 'rename reports' step
Added condition to rename files when the reports otherwise provide an apt message
Added if condition for merging the reports when the report exist otherwise provide an apt message

Issue that this pull request solves

Closes: FXA-9885 FXA-9979

Checklist

Put an x in the boxes that apply

[x] My commit is GPG signed.
[ ] If applicable, I have modified or added tests which pass locally.
[ ] I have added necessary documentation (if appropriate).
[ ] I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

Trinaa commented 1 month ago

Not sure if these things should be in scope in this fix, but we should probably consider the following:

The HTML report doesn't get generated in the case where the functional tests fail:
CircleCI documentation seems to indicate that there are 'cost implications' to storing artifacts. I wonder if we should omit artifacting the blobs and just upload the report? Or at least stop uploading it three times ; In Functional Tests - Playwright we upload the original blobs and the renamed blobs and in Merge Playwright Reports we upload the merged blob-report/report.zip, but it's all the same data. The traces are also getting duplicated.

cc: @pdehaan

ashrivastava-qa commented 1 month ago

I am thinking we should create a separate ticket for the things you have mentioned @Trinaa and merge this change so that it unblocks the PRs and the use of 're-run from failed' option. What do you think @pdehaan ?

ashrivastava-qa commented 1 month ago

@Trinaa Yes I can see and try to minimize the upload. Also we retain the data for 90 days only due to CircleCI limitation and the doc mentions "Retaining an artifact for a long period of time will have storage cost implications, therefore, it is best to determine why you are retaining artifacts" but I can look into it more to see what the cost looks like.

ashrivastava-qa commented 4 weeks ago

@Trinaa and @pdehaan I created a separate issue to address both the above mentioned items Kat pointed out. I think if this looks okay we can merge this for now to unblock the team and I am working on the other ticket separately.

ashrivastava-qa commented 3 weeks ago

@Trinaa @pdehaan

I was able to fix the script so that report-[0-7].zip generates even for the failed test machine.
For minimizing the artifacts upload, we are uploading the report.zip file thrice (twice in Functional-Test Playwright job and once in Merge Reports job). However the problem I am encountering when trying to minimize are:

a. If I remove the blob report directory path from Playwright.config.ts file, which is duplicating the reports.zip file in all the machines, no reports.zip file generates at all. Couldn't understand why as the Playwright docs suggests its either the path needs to be setup or the Environment variable but somehow we have to use path with or without Env variables.

b. The second upload is just renaming (not copying) the report.zip file to report-[0-7].zip file and store in artifacts/blob-report so that it can be merged.

c. The last upload is the merged file again in artifacts/blob-report for Merge Reports job to be able to use it for the HTML reporting. And this is required can’t not skip this upload.

d. Tried to remove the 'rename reports' step altogether and use PLAYWRIGHT_BLOB_OUTPUT_FILE="report-$(CIRCLE_NODE_INDEX).zip in config.yaml and playwright.config.ts using the output filename but that didn't work either.

So, IMO the last 2 steps are required but the first step could be removed which is the cause of duplication but somehow removing it doesn't generate the reports file at all. And the CircleCI doc states “Retaining an artifact for a long period of time will have storage cost implications, therefore, it is best to determine why you are retaining artifacts” and we only retain the data for 90days so I am guessing it won’t be too costly and I talked to Ben as well and he mentioned in Nimbus/experimenter he has seen the cost to be minimal for artifacts upload as close to .0001 cent or something. But we can't be certain, I will talk to SRE to see if they have any info on this.

So let me know if you have other thoughts on keeping the extra upload happening in step 1.

here's the POC PR

mozilla / fxa