-
Notifications
You must be signed in to change notification settings - Fork 147
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
Add the possibility to freeze a regression in a specific randomized tests scenario #1153
Conversation
… cause failures during commit
In order to recreate segfaults: SHA-532116d975aad44970092c814dc7fd17dac6d8bd $ cd tests/randomized $ make tracer.version TRACER_VERSION=0.48.2 $ make generate SEED=1234567 $ make -C .tmp.scenarios test.scenario.randomized-836242654-centos7-7.0
In order to recreate segfaults: SHA-7ec164ec77842b3516972debc9560cfc33cc5d0a $ cd tests/randomized $ make tracer.version TRACER_VERSION=0.48.1 $ make generate SEED=1234567 $ make -C .tmp.scenarios test.scenario.randomized-1775433121-centos7-7.3
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.
LGTM 👍 I just left one comment that's not a blocker for merge. Also, I didn't comb through all the regression files. Is there something I should be looking out for other than them not being empty?
Nice work @labbati! :)
file_put_contents( | ||
$destination, | ||
\str_replace( | ||
['{{scenario_name}}'], | ||
[$scenarioName], | ||
file_get_contents(__DIR__ . '/templates/.env.template') | ||
) | ||
); |
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.
This pattern appears a number of times. Perhaps there could be a generic template generator akin to the generic substitutions supported by DockerComposeFileGenerator::generate()
?
Not a blocker for merge, but just a thought for later if you agree. :)
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.
This is a very good point. I merged this PR so we can provide value sooner in master, but I also open #1155 so we can iterate there over this feedback. Let me know what do you think and let's continue the conversation there.
Thank you @SammyK
Nothing specific. Other than regressions are actually run in CI. link If they appear there, then you are sure they are either tested with at least 1000 requests or they'll fail. |
Description
This PR adds the possibility to freeze specific scenarios as regressions and to execute those regressions at every commit.
A note to the reviewer. All files in
tests/randomized/regressions
are collapsed by default in Github to facilitate code review. Now, and in the future, my advice is to just make sure that they are not empty.make analyze
will make sure to fail if no tests have been run for a regression with successful results (https://github.com/DataDog/dd-trace-php/pull/1153/files#diff-28a4837595b4bca57b0799d3d7fd12a2cec420cc0ca47b4a424cedebc3a69da8R68).Readiness checklist
[ ] (only for Members) Changelog has been added to the release document.[ ] Tests added for this feature/bug.Reviewer checklist