-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore: stop proptest failures by giving fewer guarantees #32885
Conversation
For the Aspect property tests, make some changes to make it easier to see what's going on: - Instead of just a visit log, keep a log of all actions: adding constructs, adding aspects, and visiting are now all part of the same time sequence with time steps so it's easier to reason about. - Clearly mark timed steps with `t=0`, `t=1`, etc.
The previous Aspect proptests were trying to assert ordering guarantees that the code didn't actually guarantee. This led to sporadic failures as the tests found scenarios in which the assertions were broken. In this PR, cut down on the guarantees that we make: only guarantee those things on aspect applications that exist at the start of iteration, and not on aspect applications that are added during iteration. This is not ideal, but it accurately reflects the current code and it stops intermittent failures. We will revisit tighter guarantees on aspects that are added during iteration in a later PR.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #32885 +/- ##
=======================================
Coverage 81.41% 81.41%
=======================================
Files 223 223
Lines 13721 13721
Branches 2416 2416
=======================================
Hits 11171 11171
Misses 2271 2271
Partials 279 279
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
The previous Aspect proptests were trying to assert ordering guarantees that the code didn't actually guarantee. This led to sporadic failures as the tests found scenarios in which the assertions were broken.
In this PR, cut down on the guarantees that we make: only guarantee those things on aspect applications that exist at the start of iteration, and not on aspect applications that are added during iteration.
This is not ideal, but it accurately reflects the current code and it stops intermittent failures. We will revisit tighter guarantees on aspects that are added during iteration in a later PR.
Depends-On: #32884
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license