-
Notifications
You must be signed in to change notification settings - Fork 50
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: smoke tests #1388
Merged
Merged
feat: smoke tests #1388
Changes from 15 commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
602bac1
feat: smoke tests
0marperez 989d8ec
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez 9ca8f2e
Sending verified commit to trigger CI
0marperez b00ec1f
Actually sign commit message[B
0marperez 35708df
PR feedback
0marperez 2399355
Remove error log commited by accident
0marperez 2d98a99
Add TODO
0marperez d7a4877
Added E2E tests
0marperez f390f6a
Remove dangerous gradle task, move sdk denylist check to codegen inst…
0marperez 90a93ac
Fix kotlin native builds failing
0marperez 6b288ed
Temp print statement to debug
0marperez c6701cb
Change debugging print to exception
0marperez 95b553d
chmod gradlew before tests
0marperez 5ac3db2
Use gradlew.bat
0marperez ca30211
ls sdk root dir
0marperez 08356db
Use gradle connector instead of process builder
0marperez e7fdeff
build before testing
0marperez 8f0dec1
increase gradle connection timeout
0marperez 431e63e
run from sdk root dir, better logging, use explicit gradle distributi…
0marperez 1706764
Enable gradle daemon with an idle timeout, increment internal timeout
0marperez bb53c3d
use gradle runner instead of connector
0marperez e13e9fc
Specify smokeTests to run & better test failure logging
0marperez afc5168
fix parallel task execution issues
0marperez c39f3fe
move task dependency from ci tests to code
0marperez 5dfd004
easy to fix PR feedback and decoupling SmokeTestRunnerGenerator from SDK
0marperez 87fa6bb
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez c1e0edf
Remove accidental changes to AWS model
0marperez dfcffbb
Run smoke tests in common
0marperez 25b8e8e
Self nits
0marperez 8aae6e4
Use new runtime function for env vars
0marperez 5687f7e
Cleanup, Clarity changes to test code in src
0marperez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
2 changes: 1 addition & 1 deletion
2
tests/codegen/smoke-tests/src/test/resources/smoke-tests-failure.smithy
This file contains 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
2 changes: 1 addition & 1 deletion
2
tests/codegen/smoke-tests/src/test/resources/smoke-tests-success.smithy
This file contains 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
Oops, something went wrong.
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.
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.
dependsOn
andmustRunAfter
seem redundant, why do we need both?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.
mustRunAfter
only tells gradle that if both tasks are being run then one must run after the other. But it doesn't force both tasks to run. That's whatdependsOn
is for. Not very intuitive imo.https://docs.gradle.org/8.5/kotlin-dsl/gradle/org.gradle.api/-task/must-run-after.html
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.
And when running a task with the
parallel
flag ,dependsOn
only guarantees that the task that is depended on starts first but not that it ends first. SomustRunAfter
fixes thatThere 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.
That's kind of surprising. Is that documented somewhere? Or were you seeing errors in the build without this ordering constraint?
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 don't think it's documented anywhere but I was seeing errors in the build without the ordering constraint.