Skip to content
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

Use @smithy.test#smokeTests in test models (was: Add tests to verify how AWS SDKs handle optional types) #298

Open
seebees opened this issue Oct 11, 2023 · 3 comments · Fixed by #355
Assignees

Comments

@seebees
Copy link
Contributor

seebees commented Oct 11, 2023

In the .NET SDK an optional value is stored privately as null.
There is also a private IsSet method that will identify if the value is set or not.

The .NET SDK will return never return null.
If you create a DDB input with Limit for example, but do not set the Limit value,
the underlying value will be null.
However, if you try and get this value, you will not get null you will get 0.

We should add a test model to check this.
Since it is likely to come up in other languages.

@texastony
Copy link
Contributor

Internally,
we tracked this as CrypTool-5098,
which I will now resolve in favor of this Issue.

A similar edge case was addressed in Java via #191.

Additionally,
any Test Model we add to address this
will require Native (or Extern) code.

The request MUST originate on the Native side,
and then MUST be passed to Dafny,
for us to assert that the ToDafny conversion
respects unset values.

@robin-aws
Copy link
Contributor

robin-aws commented Apr 15, 2024

A great trait for this was just added to the Smithy specification: https://smithy.io/2.0/additional-specs/smoke-tests.html#smoketests-trait

The trait defines simple requests to send to a service and an assertion that the operation either succeeds or fails. This is fairly ideal for testing constraints such as @required in test models:

@aws.polymorph#localService(
  sdkId: "SimpleConstraints",
  config: SimpleConstraintsConfig,
)
service SimpleConstraints {
  version: "2021-11-01",
  resources: [],
  operations: [ GetConstraints ],
  errors: [ SimpleConstraintsException ],
}

@smithy.test#smokeTests(
    [
        {
            id: "PresentSuccess"
            params: {mustBePresent: "No problem"}
            expect: {
                success: {}
            }
        }
        {
            id: "AbsenceError"
            params: {}
            expect: {
                failure: {}
            }
        }
    ]
)
operation GetConstraints {
  input: GetConstraintsInput,
  output: GetConstraintsOutput,
}

structure GetConstraintsInput {
  @required mustBePresent: String
}

...

The key difference between this testing method and Dafny-based tests: the params document can specify invalid data, which Dafny code can't given that we embed constraint traits into verified preconditions/non-optional types/subset types/etc.

There is extra code generation to implement to generate target language shape values. For Java, for example, the tests should become something like (not even attempting to get the smithy-generated Java API exactly right):

  @Test
  public void PresentSuccess() {
    ConstraintsClient client = ...;
    GetConstraintsInput input = GetConstraintsInput.Builder()
      .mustBePresent("No problem")
      .build();
    GetConstraintsResult result = client.GetConstraints(input);
    assertTrue(result.isSuccess());
  }

  @Test
  public void AbsenceError() {
    ConstraintsClient client = ...;
    GetConstraintsInput input = GetConstraintsInput.Builder()
      .build();
    GetConstraintsResult result = client.GetConstraints(input);
    assertTrue(result.isError());
  }

@robin-aws robin-aws self-assigned this Apr 15, 2024
@robin-aws robin-aws changed the title Add tests to verify how AWS SDKs handle optional types Use @smithy.test#smokeTests in test models (was: Add tests to verify how AWS SDKs handle optional types) Apr 17, 2024
@robin-aws
Copy link
Contributor

Renamed the title to better reflect the scope of the solution: we can test constraints effectively with this for both local services and SDKs, and really anything where Dafny-based testing isn't as effective.

robin-aws added a commit that referenced this issue May 10, 2024
Issue #, if available:

Fixes #298

Description of changes:

This trait is ideal for filling a testing gap in our TestModels. Because our strategy for mapping Smithy shapes to Dafny types enforces constraints statically, it's not possible to express invalid input without lying to Dafny through axioms that aren't actually true, or in some cases (such as @required) not possible at all. smokeTests instances generate tests that specify inputs at the Smithy level using document types (i.e. more or less JSON valued arguments to the trait), which generate target language tests using the customer-facing interfaces such as builders to create input, and can therefore express invalid input as well.

The key new component in the implementation is the ModeledShapeValue.shapeValue method, which creates a code block in Java expressing the given Shape value. Similar logic already appears in other smithy code generators, although mostly for HTTP Protocol Compliance Tests instead. I mainly imitated the structure of smithy-python's logic, but was also inspired by smithy-typescript and smithy-rs.

Because this feature introduces a new category of generated code (Smithy-generated tests) this PR also adds an --output-java-test option with SmithyDafnyMakefile.mk support. We will likely need to add a similar parameter for the other languages. It may make more sense to migrate to smithy-build.json first though: #356.

It also adds ./gradlew test as an additional step for make test_java, since running the Java @tests is distinct from executing the transpiled dafny test program.

Finally, it also improves the CLI interface to print out validation events even if there are only warnings. Besides providing previously hidden but useful information in general, this provides a way to warn against using smokeTests outside of test models - since it doesn't/can't support @reference shapes it's not terribly useful as a testing method for local services themselves.
@robin-aws robin-aws reopened this May 10, 2024
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 a pull request may close this issue.

3 participants