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

fix: remove broken check in test #3901

Merged
merged 2 commits into from
Mar 23, 2023
Merged

fix: remove broken check in test #3901

merged 2 commits into from
Mar 23, 2023

Conversation

toidiu
Copy link
Contributor

@toidiu toidiu commented Mar 22, 2023

Description of changes:

The following check in the test doesnt work as intended and fails in certain environments. The remaining code is fine and tested so we are only removing the excessive check which was added in #3853.

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

@github-actions github-actions bot added the s2n-core team label Mar 22, 2023
Copy link
Contributor

@lrstewart lrstewart left a comment

Choose a reason for hiding this comment

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

Let's also create an issue to fix this check, since it does prevent us from breaking the feature probe in the future.

@WillChilds-Klein
Copy link
Contributor

WillChilds-Klein commented Mar 22, 2023

Let's also create an issue to fix this check, since it does prevent us from breaking the feature probe in the future.

+1; the "if true" case is necessary. the "else" case is not, but I'm surprised by it failing. I'll dig into the details. please assign the issue to me.

@toidiu toidiu marked this pull request as ready for review March 22, 2023 21:37
@toidiu toidiu enabled auto-merge (squash) March 22, 2023 21:38
@toidiu
Copy link
Contributor Author

toidiu commented Mar 22, 2023

Let's also create an issue to fix this check, since it does prevent us from breaking the feature probe in the future.

+1; the "if true" case is necessary. the "else" case is not, but I'm surprised by it failing. I'll dig into the details. please assign the issue to me.

Created ticket #3902

@toidiu toidiu merged commit c9588d0 into aws:main Mar 23, 2023
@toidiu toidiu deleted the ak-rm_broken_test branch March 23, 2023 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants