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

Test CLI startup time with CI/CD job #706

Merged
merged 1 commit into from
May 22, 2023
Merged

Conversation

juhoinkinen
Copy link
Member

@juhoinkinen juhoinkinen commented May 19, 2023

Adds a GitHub Actions job with a shell script for measuring the CLI startup time to prevent its unnoticed deterioration.

The startup time was optimized in #696, and now it is good make sure the startup remains fast. For example, I noticed that adding type hints (planned in #690) can slow the startup, but this can be avoided by postponing the evaluation of annotations.

The timing script executes annif --help four times and measures user + sys time. If the average of the user + sys time is over a threshold, the job fails. The threshold is set to 0.300 s.

In the five previous runs the measured time was 0.245 s, 0.240 s, 0.252 s, 0.242 s and 0.245 s, but in some earlier runs the measured time was over 0.300 s. So failures can be expected, however I think it does not matter if they are reasonably rare (say ~10% of runs).

Failure of this job does not make the other jobs fail, so unit testing, linting and publishing are run in any case.

@juhoinkinen juhoinkinen added this to the 1.0 milestone May 19, 2023
@sonarcloud
Copy link

sonarcloud bot commented May 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (6f13121) 99.66% compared to head (881c270) 99.66%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #706   +/-   ##
=======================================
  Coverage   99.66%   99.66%           
=======================================
  Files          89       89           
  Lines        6295     6295           
=======================================
  Hits         6274     6274           
  Misses         21       21           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@juhoinkinen juhoinkinen marked this pull request as ready for review May 19, 2023 09:31
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

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

LGTM, very nice guard against slow imports creeping in!
I gave a minor suggestion as a comment.

average_startup_time=$(echo "scale=3; ($startup_time1 + $startup_time2 + $startup_time3 + $startup_time4) / 4" | bc)

# Print the average startup time
echo "Average Startup time: $average_startup_time seconds"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe print the threshold here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is enough to print the threshold only if it is exceeded, so I'll let this be as is.

@juhoinkinen juhoinkinen merged commit 4f6994b into main May 22, 2023
@juhoinkinen juhoinkinen deleted the startup-timing-test branch May 22, 2023 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants