test(aws-serverless): Split npm & layer tests#20442
Open
Conversation
Contributor
size-limit report 📦
|
mydea
force-pushed
the
fn/split-aws-serverless-test
branch
from
6779954 to
039637f
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
15
to
25
| "@playwright/test": "~1.56.0", | ||
| "@sentry-internal/test-utils": "link:../../../test-utils", | ||
| "@sentry/aws-serverless": "link:../../../../packages/aws-serverless/build/aws/dist-serverless/", | ||
| "@sentry/aws-serverless": "file:../../packed/sentry-aws-serverless-packed.tgz", | ||
| "aws-cdk-lib": "^2.210.0", | ||
| "constructs": "^10.4.2", | ||
| "glob": "^11.0.3", | ||
| "rimraf": "^5.0.10" | ||
| }, | ||
| "volta": { | ||
| "extends": "../../package.json" | ||
| }, | ||
| "//": "We need to override this here again to ensure this is not overwritten by the test runner", | ||
| "pnpm": { | ||
| "overrides": { | ||
| "@sentry/aws-serverless": "link:../../../../packages/aws-serverless/build/aws/dist-serverless/" | ||
| } | ||
| }, | ||
| "sentryTest": { | ||
| "variants": [ | ||
| { | ||
| "build-command": "NODE_VERSION=22 pnpm test:build", | ||
| "assert-command": "NODE_VERSION=22 pnpm test:assert", | ||
| "label": "aws-serverless (Node 22)" | ||
| }, | ||
| { | ||
| "build-command": "NODE_VERSION=18 pnpm test:build", | ||
| "assert-command": "NODE_VERSION=18 pnpm test:assert", | ||
| "label": "aws-serverless (Node 18)" | ||
| } | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: Removing sentryTest.variants from aws-serverless/package.json stops setting the NODE_VERSION environment variable, which will cause the test build to fail due to a runtime check.
Severity: HIGH
Suggested Fix
Restore the sentryTest.variants section in aws-serverless/package.json to ensure the NODE_VERSION environment variable is set during the CI test run. Alternatively, modify the test execution script to set NODE_VERSION for this test case.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: dev-packages/e2e-tests/test-applications/aws-serverless/package.json#L7-L25
Potential issue: The removal of the `sentryTest.variants` section from
`aws-serverless/package.json` means the `NODE_VERSION` environment variable will no
longer be set for this test application. The CI process will still include this test but
will use a default command that does not set `NODE_VERSION`. This will trigger a check
in `stack.ts` that throws an error if `process.env.NODE_VERSION` is not defined, causing
the test build to fail.
Did we get this right? 👍 / 👎 to inform future reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is on top of #20441
This PR splits the aws-serverless test app into two, aws-serverless, which tests the npm package directly, and aws-serverless-layer, which tests using the layer.
This allowed to make the test app easier to understand and follow and make the dependency graph clearer as well.
I choose to run the layer in different node versions and the npm stuff only in node 20, IMHO that should be good enough, I don't think we need to also run the npm stuff in all the node versions, so we can reduce the number of test apps a bit.