Skip to content

chore(tests): change timeout reporting in idempotency e2e tests #2074

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 3 commits into from
Feb 16, 2024

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Feb 15, 2024

Description of your changes

This PR makes slight changes to the integration test cases that assert the behavior of the Idempotency utility in the event of Lambda timeouts.

The implementation of the utility hasn't changed, but in the tests we are now expecting a different number of logs as well as a different message for functions that timeout.

I am not 100% sure about why, also because I can't test the old behavior, but it seems that Lambda functions timing out now return a different number of logs when invoked API. This resembles changes that were made by the new-ish Advanced Logging Configuration feature by Lambda, but as I said I'm not entirely sure.

Given that the implementation is the same and the changes are not on our side nor in our tests but rather in the way Lambda logs a timeout event, I decided to not investigate further and use the time elsewhere.

Related issues, RFCs

Issue number: #2076

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my change is effective and works
  • The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi self-assigned this Feb 15, 2024
@dreamorosi dreamorosi requested a review from a team February 15, 2024 11:29
@boring-cyborg boring-cyborg bot added the tests PRs that add or change tests label Feb 15, 2024
@pull-request-size pull-request-size bot added the size/M PR between 30-99 LOC label Feb 15, 2024

This comment was marked as resolved.

@github-actions github-actions bot added do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels Feb 15, 2024
@dreamorosi dreamorosi linked an issue Feb 15, 2024 that may be closed by this pull request
2 tasks
@dreamorosi dreamorosi removed do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels Feb 15, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
26.5% Duplication on New Code

See analysis details on SonarCloud

@dreamorosi dreamorosi requested review from hjgraca and am29d February 15, 2024 14:05
@sthulb sthulb merged commit e5630d4 into main Feb 16, 2024
@sthulb sthulb deleted the chore/idempotency/tests branch February 16, 2024 07:12
dreamorosi added a commit that referenced this pull request Feb 16, 2024
am29d pushed a commit that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M PR between 30-99 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maintenance: investigate failing Idempotency integration tests
2 participants