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

test: [M3-6855, M3-6876] - Add script to generate JUnit test summaries #9998

Merged
merged 23 commits into from
Jan 11, 2024

Conversation

jdamore-linode
Copy link
Contributor

@jdamore-linode jdamore-linode commented Dec 14, 2023

Description 📝

This is an experimental PR that adds a yarn junit:summary package script. The purpose of this script is to read some JUnit test result XML files and output a summary of the results in different formats. My goal is to integrate this with the Jenkins pipeline to facilitate Slack notifications and GitHub Comments.

Usage

Install dev dependencies and run yarn junit:summary <path to JUnit files> --format <format>

Example:

yarn junit:summary ./packages/manager/cypress/results --format slack

Changes 🔄

  • Add yarn junit:summary package script to output test result summary in various formats

Preview 📷

Copy/pasted the output of the script to give an idea of what the output will look like. These are slightly out of date at this point but still give a pretty good idea of what the output looks like:

Slack notification preview:

Passing Failing
Screenshot 2023-12-13 at 8 50 50 PM Screenshot 2023-12-13 at 8 32 48 PM

GitHub comment preview:

Passing Failing
Screenshot 2023-12-13 at 8 32 39 PM Screenshot 2023-12-13 at 8 31 36 PM

How to test 🧪

  1. Install new dependencies with yarn
  2. Generate some JUnit XML files by running the Cypress tests using the following command:
    CY_TEST_JUNIT_REPORT=1 yarn cy:run
    (A JUnit file gets created for each .spec.ts file, so you don't have to run the entire suite to test this)
  3. Run yarn junit:summary ./packages/manager/cypress/results --format slack to output a summary of the test result files formatted for Slack
  4. Run yarn junit:summary ./packages/manager/cypress/results --format github to output a summary of the test result files formatted for GitHub

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

Copy link

github-actions bot commented Dec 14, 2023

Coverage Report:
Base Coverage: 79.87%
Current Coverage: 79.85%

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Looks good, that's pretty intense! - worked locally great

have you thought of sanitization so we don't end up with new codeQL errors? At first sight it looks fine but worth asking now

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

This will be super nifty. Thank you Joe!

It would be super neat if this used the same comment as the coverage report to minimize extra comments on the PR. That may be more trouble than its worth though.

@jdamore-linode
Copy link
Contributor Author

jdamore-linode commented Jan 2, 2024

Looks good, that's pretty intense! - worked locally great

have you thought of sanitization so we don't end up with new codeQL errors? At first sight it looks fine but worth asking now

Good thought, thanks @abailly-akamai! I'll think on this a bit and plan to push some additions tomorrow

Edit to add: in addition to the above, in case any unanticipated CodeQL errors do arise from this, I'll be happy to take a second pass to address those 👍

This will be super nifty. Thank you Joe!

It would be super neat if this used the same comment as the coverage report to minimize extra comments on the PR. That may be more trouble than its worth though.

Thanks @bnussman-akamai! I'll definitely think about that going forward -- the main complication would be that the test run data is generated in Jenkins during the e2e run while the test coverage comment is posted via GitHub Actions, so I think we'd need some trickery to bridge that gap.

When I get around to updating the pipeline to post the comment (which I'll handle separately from the Slack notification), I'll spend some time trying to figure out a way to make that work!

@jdamore-linode
Copy link
Contributor Author

@abailly-akamai Added some string escaping for the GitHub formatter to avoid the risk of users injecting HTML into GitHub comments (I don't think this would be possible anyway since the info being escaped comes from Jenkins and isn't derived from any user-created string that I'm aware of, like a branch name or git commit message, etc., and it's behind our whitelist)

Before After
Screenshot 2024-01-11 at 12 53 01 PM Screenshot 2024-01-11 at 12 53 13 PM

I'm not sure it'll make any difference as far as CodeQL is concerned, but I'll check out the alerts on our next release and if there are any I'll write some tickets to address!

@jdamore-linode jdamore-linode merged commit 654e730 into linode:develop Jan 11, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants