Skip to content

DEVSU-3011 Missing Canned Variant Text - #117

Merged
bnguyen-bcgsc merged 14 commits into
developfrom
bugfix/DEVSU-3011-missing-canned-variant-text
Sep 16, 2026
Merged

bnguyen-bcgsc merged 14 commits into
developfrom
bugfix/DEVSU-3011-missing-canned-variant-text

Conversation

@bnguyen-bcgsc

@bnguyen-bcgsc bnguyen-bcgsc commented Aug 29, 2026

Copy link
Copy Markdown
Contributor
  • DEVSU-3011
  • Update get_ipr_analyst_comments method with 'projects' key in queried variant-text being a list instead of single value (since projects and variant texts now have many-to-many relationship DEVSU-2864 Allow multiple projects to be associated with one variant-text item pori_ipr_api#499)
  • Update project name matching logic to new projects data structure
  • Update summary test to include integrated test to IPR API to validate mock IPR result schema with actual variant text schema from IPR

Requires IPR API PR: bcgsc/pori_ipr_api#516

- Update get_ipr_analyst_comments method with 'projects' key in queried variant-text being a list instead of single value (since projects and variant texts now have many-to-many relationship bcgsc/pori_ipr_api#499)
- Update project name matching logic to new projects data structure
@bnguyen-bcgsc bnguyen-bcgsc self-assigned this Aug 29, 2026
@bnguyen-bcgsc bnguyen-bcgsc added the bug Something isn't working label Aug 29, 2026
@codecov-commenter

codecov-commenter commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.84%. Comparing base (69f0e81) to head (6b27d1d).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #117      +/-   ##
===========================================
- Coverage    84.04%   83.84%   -0.20%     
===========================================
  Files           18       18              
  Lines         2613     2613              
===========================================
- Hits          2196     2191       -5     
- Misses         417      422       +5     
Flag Coverage Δ
unittests 83.84% <100.00%> (-0.20%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mathieulemieux

Copy link
Copy Markdown
Member

Fixing the unit tests is a good place to start but I was wondering why the integration tests didn't catch that bug in the first place. @elewis2, do you remember why we 'mock iprconn for ipr comments' ? 9ea52db
Was it a temporary mesure until the API gets updated, so now we can unmock it?

- Integrate IprConnection in mock result outputs so changes in IPR schema would fail tests here
@mathieulemieux

Copy link
Copy Markdown
Member

Just to keep track of things. As discussed about integration tests, maybe add a variant-text/schema route on the api side, then compare expected format with actual schema.

…nt text schema

- Validate schema generated by mock ipr results with actual schema from IPR API
@elewis2

elewis2 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Fixing the unit tests is a good place to start but I was wondering why the integration tests didn't catch that bug in the first place. @elewis2, do you remember why we 'mock iprconn for ipr comments' ? 9ea52db Was it a temporary mesure until the API gets updated, so now we can unmock it?

IMO the place an integration test would go would be in something like test_upload. But we haven't been running that from github iirc because it takes too long.

@bnguyen-bcgsc
bnguyen-bcgsc merged commit b72ef41 into develop Sep 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants