Skip to content

Protocol tests generate suites with request input setting #3376

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

Merged
merged 2 commits into from
Apr 10, 2025

Conversation

SergeyRyabinin
Copy link
Contributor

@SergeyRyabinin SergeyRyabinin commented Apr 9, 2025

Issue #, if available:
Protocol tests WIP
Description of changes:
Continue working on the protocol tests generation.
This change adds generation of test suites with tests from the model, with client creation and request input filling.

This change follows 2 ideas:

  • Uses gtest disguised as our protocol test framework, gtest maybe replaced later as we have not yet defined the behavior of test executor. So far these tests will act as unit/integration tests.
  • Convert input c2j protocol test model tree into an intermediate model more close to the original Smithy model;
  • Use recursion to fill the input data. Recursion uses c2j client model to choose the exact cpp type because input json provided by protocol test is just a json.

Note: 1 test client has been removed as the originally added c2j test client model definition was corrupted in source and added by mistake.
Note: No response validation / no http mocking in this PR.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

SergeyRyabinin added a commit that referenced this pull request Apr 9, 2025
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: 1 test client has been removed as the originally added c2j test client model definition was corrupted in source and added by mistake.

SergeyRyabinin added a commit that referenced this pull request Apr 9, 2025
@SergeyRyabinin SergeyRyabinin marked this pull request as ready for review April 9, 2025 17:22
@@ -22,20 +24,21 @@
import java.io.IOException;
import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: no star imports

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aahh, default IDE config.

}
}

public ProtocolTestModel convert(C2jTestSuite c2jTestSuite) {
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of creating a seperate Transformer class maybe it would be a good idea to call a function .from() on ProtocolTestModel that takes a C2J test suite as a model.

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 just followed the existing pattern within the project.

/**
* A JSON hash representing the structure of the parsed response. Either this or error (not both) must appear in the test case.
*/
private Optional<JsonNode> result;
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to model it? raw JsonNode should only be used if we cannot model it as a object.

@SergeyRyabinin SergeyRyabinin merged commit 61eb504 into main Apr 10, 2025
3 of 4 checks passed
@SergeyRyabinin SergeyRyabinin deleted the sr/pt/gen branch April 10, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants