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

Update protocol tests #2988

Merged
merged 174 commits into from
Apr 15, 2024
Merged

Update protocol tests #2988

merged 174 commits into from
Apr 15, 2024

Conversation

jterapin
Copy link
Contributor

@jterapin jterapin commented Feb 23, 2024

Objectives

  • Replace existing protocol-tests with Smithy's definition of protocol-tests
  • Update the test runner to consume these accordingly (and clean up)
  • Resolve test failures
  • Implement a mechanism to skip test cases (and have a log of skipped cases with reasons why)
  • Carry over protocol-tests that was not covered by Smithy's definition (i.e. eventstreams)
  • Update existing protocol-tests (api-gateway, carry overs) to include descriptive format

Starting total of failures: 349
Current total of failures: 0 but 2 ignored due to compatibility

Current standing:

  • All protocol-tests passes but more work required:
    • We have 58 ignored tests We just have 1 ignore test that we need to keep due to limitations on the JSON Engine
    • 11 tests cases rely on Smithy fixing the C2J translation (may require additional work after)
    • 4 test cases cannot be fixed due complex issue
    • 43 test cases to be resolved when test runner is updated to handle error cases (need to implement)
    • There are 6 failures in our gem specs after making changes (these need to be investigated and resolve)
    • There are 44 failures in the on build_tools directory (need to investigate and resolve)
  • Clean up code (optimize approaches, etc)
  • Create a comprehensive log of changes (intended for changelog)
  • Create a list of services to manually test certain operations that are affected by some SDK core changes
  • Manual testing/run integration tests MANUAL TESTING IN PROGRESS
  • Bring over "extra" test cases (aka eventstreams) and update the formatting
  • Update api-gateway protocol-tests formatting if needed
  • CR review and update based on feedback

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

  1. To make sure we include your contribution in the release notes, please make sure to add description entry for your changes in the "unreleased changes" section of the CHANGELOG.md file (at corresponding gem). For the description entry, please make sure it lives in one line and starts with Feature or Issue in the correct format.

  2. For generated code changes, please checkout below instructions first:
    https://github.com/aws/aws-sdk-ruby/blob/version-3/CONTRIBUTING.md

Thank you for your contribution!

Copy link

You have made a change to core without a corresponding change to the CHANGELOG.md. This change will not result in a new version and will not published unless an entry is added to CHANGELOG.md

@jterapin
Copy link
Contributor Author

Current total of failures: 252

@aws aws deleted a comment from github-actions bot Feb 28, 2024
@jterapin
Copy link
Contributor Author

Current status: 197 failures, 50 ignored

@aws aws deleted a comment from github-actions bot Feb 29, 2024
@aws aws deleted a comment from github-actions bot Feb 29, 2024
@aws aws deleted a comment from github-actions bot Mar 1, 2024
@jterapin
Copy link
Contributor Author

jterapin commented Mar 1, 2024

Current status: 160 failures, 51 ignored

@aws aws deleted a comment from github-actions bot Apr 4, 2024
@aws aws deleted a comment from github-actions bot Apr 5, 2024
@aws aws deleted a comment from github-actions bot Apr 5, 2024
Copy link
Contributor

@mullermp mullermp left a comment

Choose a reason for hiding this comment

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

Nice!

I think we need to verify presigned urls, error cases, and general backwards compatibility. The content-type should not be blocked (see comment below).

I think that after those are confirmed and fixed, any other tests that are skipped can be done as follow ups? That makes review a bit easier too.

@aws aws deleted a comment from github-actions bot Apr 9, 2024
@aws aws deleted a comment from github-actions bot Apr 10, 2024
Copy link
Contributor

@alextwoods alextwoods left a comment

Choose a reason for hiding this comment

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

Nice!! These look good to me

@aws aws deleted a comment from github-actions bot Apr 12, 2024
@jterapin jterapin merged commit 09ee4c3 into version-3 Apr 15, 2024
28 checks passed
@jterapin jterapin deleted the update_protocol_tests branch April 15, 2024 14:57
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