Skip to content

Conversation

@jpinkney-aws
Copy link
Contributor

@jpinkney-aws jpinkney-aws commented Dec 17, 2024

Problem

  • CI test reporting only captures the last package's results
  • Running npm run testE2E only preserves toolkit's report.xml, losing other package results

Solution

  • Generate individual report.xml files per subproject
  • Consolidate all existing package reports into root .test-reports/report.xml
  • This is done in the buildspecs themselves rather than directly in the package.json (the original approach to solve this problem) because mac/linux and windows have two different ways of getting the last error code, resulting in a complicated package.json for something thats only needed in our codebuilds

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Problem:
- CI test reporting only captures the last package's results
- Running `npm run testE2E` only preserves toolkit's report.xml, losing other package results

Solution:
- Generate individual report.xml files per subproject
- Consolidate all existing package reports into root .test-reports/report.xml
@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

@github-actions
Copy link

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.

@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

@Hweinstock
Copy link
Contributor

What are the tradeoffs for merging the report versus having one per extension? Curious because for code coverage we have separate ones.

@jpinkney-aws
Copy link
Contributor Author

If you don't merge them you can't see them in the ui here: https://d1ihu6zq92vp9p.cloudfront.net/487310e6-ebfc-4fef-a0b1-b441147b3c23/report.html. That's really the issue i'm trying to solve. It's pretty annoying when an amazon q test fails and then you have to manually look through the logs to find the problem

I don't think that restriction applies to codecov since you can upload multiple different reports at the same time

@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

1 similar comment
@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

@jpinkney-aws jpinkney-aws marked this pull request as ready for review December 20, 2024 14:53
@jpinkney-aws jpinkney-aws requested a review from a team as a code owner December 20, 2024 14:53
. buildspec/shared/common.sh
2>&1 xvfb-run npm test --silent | run_and_report 2 \
2>&1 xvfb-run npm test --silent; npm run mergeReports -- "$?" | run_and_report 2 \
'rejected promise not handled' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on a local test of this command:

2>&1 echo aaa; echo bbb | grep x

I suspect the above ; may not correctly pipe all the output from npm test to run_and_report as intended.

To confirm that the piping to run_and_report is still working correctly, can you try temporarily changing this search patter to something that always outputs, like:

Suggested change
'rejected promise not handled' \
'deleteTestTempDirs' \

then CI should fail.

If it doesn't, we can fix the issue by changing the pipeline to something like (note: whitespace near the braces is required):

 { 2>&1 xvfb-run npm test --silent; npm run mergeReports -- "$?"; } | run_and_report 2 \

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just tested and that was indeed the case. I've switched to your suggestion and it seems to work as expected

@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

@jpinkney-aws
Copy link
Contributor Author

/runIntegrationTests

@jpinkney-aws jpinkney-aws merged commit 42c8900 into aws:master Jan 6, 2025
8 of 17 checks passed
@jpinkney-aws jpinkney-aws deleted the merge-reports branch January 6, 2025 17:32
karanA-aws pushed a commit to karanA-aws/aws-toolkit-vscode that referenced this pull request Jan 17, 2025
## Problem
- CI test reporting only captures the last package's results
- Running `npm run testE2E` only preserves toolkit's report.xml, losing
other package results


## Solution
- Generate individual report.xml files per subproject
- Consolidate all existing package reports into root
.test-reports/report.xml
- This is done in the buildspecs themselves rather than directly in the
package.json (the original approach to solve this problem) because
mac/linux and windows have two different ways of getting the last error
code, resulting in a complicated package.json for something thats only
needed in our codebuilds


---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
s7ab059789 pushed a commit to s7ab059789/aws-toolkit-vscode that referenced this pull request Feb 19, 2025
## Problem
- CI test reporting only captures the last package's results
- Running `npm run testE2E` only preserves toolkit's report.xml, losing
other package results


## Solution
- Generate individual report.xml files per subproject
- Consolidate all existing package reports into root
.test-reports/report.xml
- This is done in the buildspecs themselves rather than directly in the
package.json (the original approach to solve this problem) because
mac/linux and windows have two different ways of getting the last error
code, resulting in a complicated package.json for something thats only
needed in our codebuilds


---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
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