-
Notifications
You must be signed in to change notification settings - Fork 412
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: set the default of _should_skip to false #11274
base: main
Are you sure you want to change the base?
Conversation
…st after the flaky time is up.
|
Datadog ReportBranch report: ✅ 0 Failed, 958 Passed, 328 Skipped, 22m 53.01s Total duration (13m 53.72s time saved) |
if condition is not None and condition: | ||
return True | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if condition is not None and condition: | |
return True | |
return False | |
return condition is not None and condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit suggestion
BenchmarksBenchmark execution time: 2024-11-14 22:41:19 Comparing candidate commit ebfb726 in PR branch Found 2 performance improvements and 0 performance regressions! Performance is the same for 386 metrics, 2 unstable metrics. scenario:iast_aspects-ospathjoin_aspect
scenario:iast_aspects-ospathnormcase_aspect
|
While reviewing the flaky marker in gunicorn's
test_span_schematization
indd-trace-py/tests/contrib/gunicorn/test_gunicorn.py
Line 188 in 223c261
ie:
This translates to Jan 31, 2024.
However, if you run the test for
test_span_schematization
today, it passes fine because it is getting skipped:The bug fix here is to set the default to false, but keep the spirit of the original
condition
if statement.If you rerun the test_span_schematization with this branch, it now doesn't get skip and the test correctly fails:
Leaving this PR in a draft state for now to see what other tests this affects.
Checklist
Reviewer Checklist