Add generator to generate tests that service client headers are buildable #3391
Conversation
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
11 tasks
SergeyRyabinin
added a commit
that referenced
this pull request
Apr 24, 2025
SergeyRyabinin
force-pushed
the
sr/genIncludeTest
branch
from
2e09fd8 to
8d965af
Compare
SergeyRyabinin
added a commit
that referenced
this pull request
Apr 28, 2025
SergeyRyabinin
force-pushed
the
sr/genIncludeTest
branch
3 times, most recently
from
45fef39 to
3c54f1c
Compare
SergeyRyabinin
commented
Apr 29, 2025
| cfgInit.shouldDisableIMDS = true; | ||
| Aws::Client::ClientConfiguration config(cfgInit); | ||
| AWS_UNREFERENCED_PARAM(config); | ||
| // auto pClient = Aws::MakeUnique<{{ t_namespace }}::{{ t_cpp_name }}>("{{ t_test_name }}", config); |
There was a problem hiding this comment.
commented out because CodeCatalyst client is not default constructible.
TODO: fix that client or this test generation.
SergeyRyabinin
force-pushed
the
sr/genIncludeTest
branch
from
3c54f1c to
b3704c6
Compare
sbiscigl
approved these changes
Apr 29, 2025
|
|
||
| for root, dirs, files in os.walk(service_client_include_dir): | ||
| for filename in files: | ||
| if filename.endswith(".h") or filename.endswith(".hpp"): |
There was a problem hiding this comment.
do any of our files actually end with .hpp?
There was a problem hiding this comment.
nope, but if ever anyone is going to add an .hpp - this generator will pick them up.
| with open(f"{test_file_path}", mode="w", encoding="utf-8") as rendered_test_f: | ||
| rendered_test_f.write(rendered_test) | ||
|
|
||
| return 0 |
There was a problem hiding this comment.
nit: why return 0 if we dont return any other value? maybe prefer void
There was a problem hiding this comment.
just following the pattern of "generator return 0" on success, and throws an exception on error (and something such as return -1 in the middle).
SergeyRyabinin
force-pushed
the
sr/genIncludeTest
branch
from
b3704c6 to
109dde0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue #, if available:
#3389
Description of changes:
Prevent such issues by a include all headers test.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.