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

Add perf-guard github workflow #345

Merged
merged 31 commits into from
Dec 9, 2024
Merged

Add perf-guard github workflow #345

merged 31 commits into from
Dec 9, 2024

Conversation

sirbrillig
Copy link
Owner

@sirbrillig sirbrillig commented Dec 2, 2024

This adds a new Github action CI check to make sure that the performance of the sniff does not jump wildly high.

It uses the PHPMailer.php file from here: https://raw.githubusercontent.com/PHPMailer/PHPMailer/refs/tags/v6.9.3/src/PHPMailer.php as a test fixture, since that file is relatively large and complex. It then sets a threshold for scanning that file with this sniff.

Fixes #343

.github/workflows/csqa.yml Outdated Show resolved Hide resolved
.github/workflows/csqa.yml Outdated Show resolved Hide resolved
.github/workflows/csqa.yml Outdated Show resolved Hide resolved
.github/workflows/csqa.yml Outdated Show resolved Hide resolved
@sirbrillig
Copy link
Owner Author

Nice, I need to pick a better target file but this does work. You can see the failure in 769bd71 when I reduced to baseline time to 0.2.

@jrfnl
Copy link
Collaborator

jrfnl commented Dec 3, 2024

Was just thinking - I imagine that as things are now, it may be hard to debug the workflow when it fails as it may not be clear on what timing it failed.
Could be an idea to:

  • first run the report in a separate step and store the report to $GITHUB_OUTPUT.
  • In a next step to echo out the report for debug purposes
  • Then in the step after that use grep to get the timing target from the report.

@sirbrillig
Copy link
Owner Author

Good thinking! I got that to work, I believe, although I still had to collect the output, print it, and grep it in the same step because I can't figure out how to pass a multi-line github output variable through grep. I figured out how to store the multi-line output in an output variable but then echoing it back into a unix pipe seems very difficult 🤔 . Hopefully a single step will be ok for now.

@sirbrillig sirbrillig marked this pull request as ready for review December 8, 2024 19:51
@sirbrillig
Copy link
Owner Author

although I still had to collect the output, print it, and grep it in the same step because I can't figure out how to pass a multi-line github output variable through grep

I cheated and dumped the output to a file. That works.

@sirbrillig sirbrillig merged commit 6a0023c into 2.x Dec 9, 2024
64 checks passed
@sirbrillig sirbrillig deleted the add-perf-ci-guard branch December 9, 2024 01:45
@jrfnl
Copy link
Collaborator

jrfnl commented Dec 9, 2024

@sirbrillig Nicely done ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Needs performance guard CI process
3 participants