-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(synthetics): support runtime nodejs puppeteer 4.0 #25553
feat(synthetics): support runtime nodejs puppeteer 4.0 #25553
Conversation
…at/synthetics/support-runtime-puppeteer-4.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This blueprint was renewed in AWS Synthetics service console and failed run, so I replaced this script to latest script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
conn.close() | ||
|
||
|
||
def main(): | ||
|
||
url = 'https://example.com/' | ||
url = os.environ['URL'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this dummy url to actual URL that created in integ test stack.
// Assertion that all Canary's are Passed | ||
[ | ||
inlineAsset, | ||
directoryAsset, | ||
zipAsset, | ||
puppeteer39, | ||
puppeteer40, | ||
selenium13, | ||
].forEach((canary) => test.assertions | ||
.awsApiCall('Synthetics', 'getCanaryRuns', { | ||
Name: canary.canaryName, | ||
}) | ||
.assertAtPath('CanaryRuns.0.Status.State', ExpectedResult.stringLikeRegexp('PASSED')) | ||
.waitForAssertions({ totalTimeout: cdk.Duration.minutes(5) })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assertion can test that status of canaries is passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This blueprint was renewed in AWS Synthetics service console and failed run, so I replaced this script to latest script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by this? does the old canary runtime code not work? We intentionally did not use AWS Synthetics blueprints because we don't want to be in charge of making sure that our copy is the same as the blueprint.
If the current code fails, i would prefer to fix it in the most basic way possible than replace it with the actual blueprint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. old canary runtime code was not work. I got failed status.
However, I understood your think, so I changed runtime code to close to minimum changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the new runtime and updating the integ test! My only comments are about our test canaries themselves -- they are intentionally not supposed to mirror any blueprints that AWS Synthetics provides
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by this? does the old canary runtime code not work? We intentionally did not use AWS Synthetics blueprints because we don't want to be in charge of making sure that our copy is the same as the blueprint.
If the current code fails, i would prefer to fix it in the most basic way possible than replace it with the actual blueprint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Thanks @kaizencc for your reviewing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @WinterYukky!
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
What change
Add CloudWatch Synthetics runtime nodejs puppeteer 4.0.
Others
I changed integ test to using integ-tests assertions.
Closes #25493
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license