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: performance test CI work #8761

Merged
merged 42 commits into from
Feb 1, 2024
Merged

test: performance test CI work #8761

merged 42 commits into from
Feb 1, 2024

Conversation

NicholasBlaskey
Copy link
Contributor

@NicholasBlaskey NicholasBlaskey commented Jan 26, 2024

Description

Does the infra work for perf tests.

Runs the performance tests on every commit on main and allows running on feature branches.

Also records migration timings. Writes this all to a postgres instance we will eventually hook up to our ci grafana.

Test Plan

Manual, and also we will test a bunch after this lands and adjust any issues we find.

Commentary (optional)

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

@cla-bot cla-bot bot added the cla-signed label Jan 26, 2024
Copy link

netlify bot commented Jan 26, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 35ba388
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/65bbf4cf9af4990008a5892c

@NicholasBlaskey NicholasBlaskey changed the title first step test: performance test CI work Jan 26, 2024
Copy link

codecov bot commented Jan 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ea9e903) 47.44% compared to head (35ba388) 47.44%.
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8761      +/-   ##
==========================================
- Coverage   47.44%   47.44%   -0.01%     
==========================================
  Files        1046     1046              
  Lines      166888   166888              
  Branches     2239     2241       +2     
==========================================
- Hits        79185    79182       -3     
- Misses      87545    87548       +3     
  Partials      158      158              
Flag Coverage Δ
backend 41.83% <ø> (-0.01%) ⬇️
harness 64.36% <ø> (ø)
web 42.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 6 files with indirect coverage changes

performance/Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@stoksc stoksc left a comment

Choose a reason for hiding this comment

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

looks great

command: python .circleci/scripts/wait_for_perf_migration_upload_results.py

- when:
condition: <<parameters.take-snapshot>>
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe call this snapshot-after-migrations

),
// This is a bad endpoint and we know it is bad.
// No sense in making other endpoints be slowed before we get the fix in
// https://hpe-aiatscale.atlassian.net/browse/DET-10114
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we ensure that a ticket is created or a comment is made in one of these tickets to re-enable this test. Although I am assume that was the plan in the ticket anyway, I just don't want us to forget.

@NicholasBlaskey NicholasBlaskey merged commit 40a70cf into main Feb 1, 2024
75 of 87 checks passed
@NicholasBlaskey NicholasBlaskey deleted the perf_tests_in_ci branch February 1, 2024 20:44
maxrussell pushed a commit that referenced this pull request Mar 21, 2024
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.

4 participants