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-7844] - Add script to generate JSON payload for TOD test results #10422

Merged

Conversation

jdamore-linode
Copy link
Contributor

Description πŸ“

This adds another script in scripts/ to generate a payload for our internal test result store, TOD. See M3-7843 for the Cloud/TOD epic which provides more context and outlines our plan to integrate Cloud with TOD.

The purpose of this script is to consume a JUnit XML file (or a bunch of JUnit XML files) and output a TOD-compatible JSON payload. This payload contains the JUnit data itself, along with various metadata describing the project and test run.

I opted to add the script here because it will need to be invoked from Jenkins and from GitHub Actions, and wanted to avoid having to implement the same script twice.

Changes πŸ”„

  • Add yarn generate-tod script

How to test πŸ§ͺ

Pre-Requisites

You'll need at least one JUnit XML file to test with. The quickest way to generate one is to pass --reporter=junit when invoking yarn test, like this:

yarn --silent test features/Profile/APITokens/CreateAPITokenDrawer.test --reporter=junit >> junit.xml

Alternatively, you can generate JUnit files using Cypress:

CY_TEST_JUNIT_REPORT=1 yarn cy:run -s "cypress/e2e/core/account/account-cancellation.spec.ts"

The JUnit files will be output to cypress/results.

Testing

Confirm that yarn generate-tod script works as expected. Run yarn generate-tod --help for a detailed breakdown of command syntax and optional parameters. Some scenarios to test:

  • Generating a payload containing a single JUnit file by passing a path to the JUnit file
  • Generating a payload containing multiple JUnit files by passing a path to a directory containing JUnit files
  • Generating a payload containing extra metadata using --appName, --appBuild, --appBuildUrl, --fail, --tag, and related parameters

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

@jdamore-linode jdamore-linode self-assigned this Apr 30, 2024
@jdamore-linode jdamore-linode requested a review from a team as a code owner April 30, 2024 18:00
@jdamore-linode jdamore-linode requested review from dwiley-akamai and carrillo-erik and removed request for a team April 30, 2024 18:00
Copy link

github-actions bot commented Apr 30, 2024

Coverage Report: βœ…
Base Coverage: 81.82%
Current Coverage: 81.82%

@@ -45,6 +45,7 @@
"coverage": "yarn workspace linode-manager coverage",
"coverage:summary": "yarn workspace linode-manager coverage:summary",
"junit:summary": "ts-node scripts/junit-summary/index.ts",
"generate-tod": "ts-node scripts/tod-payload/index.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

In the future it would be nice to run these helper scripts with bun so we don't have to depend on ts-node

I guess we'd have to update our Jenkins pipelines and Github Actions to install bun first

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I doubt that'd be a big lift! I'll follow up with a ticket soon

@carrillo-erik
Copy link
Contributor

Tested the commands and was able to successfully execute them. There was a warning message, I'll paste here for your attention:

WARNING: Secure coding is not enabled for restorable state! Enable secure coding by implementing NSApplicationDelegate.applicationSupportsSecureRestorableState: and returning YES.

Also, would this be an issue for maintainability in the future?

You are running Node v20.12.2. Only the following versions of Node are supported:
  - v16.x
  - v18.x

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Ready for Review labels May 2, 2024
@jdamore-linode
Copy link
Contributor Author

Thanks @carrillo-erik, and sorry for the slow response!

Tested the commands and was able to successfully execute them. There was a warning message, I'll paste here for your attention:

WARNING: Secure coding is not enabled for restorable state! Enable secure coding by implementing NSApplicationDelegate.applicationSupportsSecureRestorableState: and returning YES.

This one's new to me! I haven't been able to reproduce it, but NSApplicationDelegate is a Cocoa API and judging from some Google results, it might be related to executing code from within another program (VSCode?). All the results I've found seem to be related to Python on MacOS 14, however, so kind of a shot in the dark. Either way, if everything is working I'd wager this isn't an issue for us.

Also, would this be an issue for maintainability in the future?

You are running Node v20.12.2. Only the following versions of Node are supported:
  - v16.x
  - v18.x

The usefulness of these warnings is debatable, but I think the intent is to let outside contributors know if they're running Cloud/the tests on a version of Node that isn't compatible with our codebase. The "supported" version is Node 18.x just because that's the version of Node we use in our test and deploy pipelines, but I'd be extremely surprised if there are compatibility issues with modern versions of Node.

I was on the fence about getting rid of this warning when we upgraded from Node 14 -- curious if you have any opinion on that? Seems like it might cause more confusion than anything else

@jdamore-linode jdamore-linode merged commit bb6c22c into linode:develop May 21, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants