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: add intg tests for tasks module #8750

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Conversation

carolinaecalderon
Copy link
Contributor

@carolinaecalderon carolinaecalderon commented Jan 24, 2024

Description

Add integration tests for all exported functions in db/postgres_tasks.go, and initialize the db in TestMain.

Add bun descriptions to the AllocationSession & TaskLog models, so that the ID key is auto-incremented with each addition.

Test Plan

See attached test.

Commentary (optional)

Truncated the start time in RequireMockAllocation to match what the db stores.
Remove CheckTaskExists from postgres_tasks.go, as it's used nowhere.

Writing the tests first, then bun-ifying db/postgres_tasks.go in #8764

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

DET-10124

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

netlify bot commented Jan 24, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit da2f941
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/65bad6ca4600f0000801ec12
😎 Deploy Preview https://deploy-preview-8750--determined-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Jan 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9ec2f7d) 47.58% compared to head (da2f941) 47.27%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8750      +/-   ##
==========================================
- Coverage   47.58%   47.27%   -0.31%     
==========================================
  Files        1049     1049              
  Lines      167354   167345       -9     
  Branches     2241     2241              
==========================================
- Hits        79633    79118     -515     
- Misses      87563    88069     +506     
  Partials      158      158              
Flag Coverage Δ
backend 43.06% <100.00%> (+0.49%) ⬆️
harness 62.55% <ø> (-1.77%) ⬇️
web 42.54% <ø> (ø)

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

Files Coverage Δ
master/internal/db/postgres_tasks.go 82.94% <ø> (+49.23%) ⬆️
master/internal/db/postgres_test_utils.go 77.43% <100.00%> (ø)
master/pkg/model/task.go 15.59% <ø> (ø)

... and 14 files with indirect coverage changes

@carolinaecalderon carolinaecalderon force-pushed the bunify-task-tests branch 5 times, most recently from 32092d7 to d1bb042 Compare January 29, 2024 19:34
@carolinaecalderon carolinaecalderon marked this pull request as ready for review January 29, 2024 19:41
@carolinaecalderon carolinaecalderon requested a review from a team as a code owner January 29, 2024 19:41
@carolinaecalderon carolinaecalderon self-assigned this Jan 29, 2024
Copy link
Contributor

@salonig23 salonig23 left a comment

Choose a reason for hiding this comment

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

A few comments but the rest looks great!

Copy link
Contributor

@NicholasBlaskey NicholasBlaskey left a comment

Choose a reason for hiding this comment

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

Looks good to me, nice work

I would wait for @salonig23 's approval too

master/internal/db/postgres_tasks_intg_test.go Outdated Show resolved Hide resolved
master/internal/db/postgres_tasks_intg_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@salonig23 salonig23 left a comment

Choose a reason for hiding this comment

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

LGTM, great work!

@carolinaecalderon carolinaecalderon merged commit ec850ae into main Feb 1, 2024
75 of 86 checks passed
@carolinaecalderon carolinaecalderon deleted the bunify-task-tests branch February 1, 2024 00:51
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.

3 participants