Y24-382: add example test cases #4641

StephenHulme · 2025-01-27T13:06:36Z

Changes proposed in this pull request

add example test cases from story
- lays the groundwork for Y24-383 - Cherrypicking improvements (minimum volume) #4413

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
- Check story numbers included
- Check for debug code
- Check version

StephenHulme · 2025-01-27T13:23:20Z

This is ready for review, I think the failing feature tests are unrelated to the changes (and possibly solved by #4639).

StephenHulme · 2025-01-27T16:29:59Z

spec/models/well_spec.rb

@@ -261,31 +261,46 @@
    end
  end

+  # TODO: Y24-383 - add assertions for final source and destination volumes


I'm half wondering if this should happen as part of this PR to lay a groundwork of completed calculations for a potential meeting with relevant parties next week...

When you say "final source and destination volumes", you mean "current_volume" in well attributes, on the source and target wells - is that right?

Great idea to add these assertions though

I'm not sure... current_volume seems somewhat time invarient to me - I'm not sure at what stage it is current! What I mean is how much volume is left in the original well on the original plate, and how much has been pipetted into the new well in the new plate.
But I think we are talking the same thing...?

Sorry should have submitted all these comments in one go...

Is it possible to check the target well's final volume in this test, because that gets updated after bed verification?

Oh.... that makes sense why it isn't in this calculation at the moment... 🙈 There are calculations for that, I'll have a look and try add it to this PR.

codecov · 2025-01-29T07:12:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.49%. Comparing base (088dcf7) to head (d1f1c5b).
Report is 37 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4641      +/-   ##
===========================================
+ Coverage    87.18%   89.49%   +2.30%     
===========================================
  Files         1406     1406              
  Lines        30296    30630     +334     
===========================================
+ Hits         26415    27412     +997     
+ Misses        3881     3218     -663

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

KatyTaylor · 2025-01-29T14:17:12Z

spec/models/well_spec.rb

+    { target_ng: 10_000, measured_conc: 250, measured_vol: 50, min_vol:10, min_pick_vol: 1, source_pick_vol: 40, buffer_vol: 0, current_vol: nil },
+    { target_ng: 10_000, measured_conc: 250, measured_vol: 30, min_vol:10, min_pick_vol: 1, source_pick_vol: 30, buffer_vol: 0, current_vol: nil },
+    { target_ng: 1000, measured_conc: 70, measured_vol: 50, min_vol:10, min_pick_vol: 5, source_pick_vol: 14.29, buffer_vol: 0, current_vol: nil },# Y24-382: SQPD-10861 v14.29, b0.00
+    { target_ng: 200, measured_conc: 200, measured_vol: 1, min_vol:50, min_pick_vol: 5, source_pick_vol: 1, buffer_vol: 49, current_vol: nil }, # Y24-382: SQPD-10864 v1.00, b49.00


So this one (above) will pass currently, but when Y24-383 is done it should be v5.00, b45.00 - is that right?

If required volume < robot minimum picking volume, round up to the robot minimum picking volume

By the logic quoted, yes I think so.

My plan is to create a number of worked examples/scenarios, and liase with the stakeholders in Y24-383 to confirm which results are expected. Then make the algorithm match those expectations - Test Driven Development for the win 😉

spec/models/well_spec.rb

StephenHulme · 2025-01-29T14:34:42Z

spec/models/well_spec.rb

@@ -437,7 +452,8 @@
      [100.0, 5.0, 100.0, 2.0, 5.0, 5.0, 98.0, 'Less DNA than robot minimum pick'],
      [100.0, 50.0, 1.0, 200.0, 5.0, 100.0, 0.0, 'Low concentration, maximum DNA, no buffer'],
      [120.0, 50.0, 0, 60.0, 5.0, 60.0, 60.0, 'Zero concentration, with less volume than required'],
-      [120.0, 50.0, 0, 3.0, 5.0, 5.0, 117.0, 'Zero concentration, with less volume than even the minimum robot pick']
+      [120.0, 50.0, 0, 3.0, 5.0, 5.0, 117.0, 'Zero concentration, with less volume than even the minimum robot pick'],
+      [15.0, 50.0, 70.0, 50, 5.0, 10.7, 5.0, 'Y24-382: SQPD-10859 v10.71 b5.00']


The v10.71, b5.00 example is a concentration example, rather than an amount example.

KatyTaylor · 2025-01-29T14:35:23Z

spec/models/well_spec.rb

@@ -437,7 +452,8 @@
      [100.0, 5.0, 100.0, 2.0, 5.0, 5.0, 98.0, 'Less DNA than robot minimum pick'],
      [100.0, 50.0, 1.0, 200.0, 5.0, 100.0, 0.0, 'Low concentration, maximum DNA, no buffer'],
      [120.0, 50.0, 0, 60.0, 5.0, 60.0, 60.0, 'Zero concentration, with less volume than required'],
-      [120.0, 50.0, 0, 3.0, 5.0, 5.0, 117.0, 'Zero concentration, with less volume than even the minimum robot pick']
+      [120.0, 50.0, 0, 3.0, 5.0, 5.0, 117.0, 'Zero concentration, with less volume than even the minimum robot pick'],
+      [15.0, 50.0, 70.0, 50, 5.0, 10.7, 5.0, 'Y24-382: SQPD-10859 v10.71 b5.00']


So this will need to be updated after Y24-383, to be v10.71, b4.29, I think? (and still 15 final volume)

KatyTaylor · 2025-02-07T14:17:29Z

spec/models/well_spec.rb

@@ -262,31 +262,184 @@
  end

  [
-    [1000, 10, 50, 50, 0, nil],
-    [1000, 10, 10, 10, 0, nil],
-    [1000, 10, 20, 10, 0, 10],


Have you lost this third scenario, where the 'current volume' (of the source well) is 10? Does the current volume of the source plate get used in the calculation? Or is it the measured volume that is checked?

Thanks for picking this up, I'll double-check!

This is an intentional change, outlined in 2aa1b7a

In the original tests, both measured_volume and current_volume (at start) are stipulated, providing some ambiguity about which value is being used in the picking algorithm. This is resolved by the estimated_volume method which I've broken out into a separate test in this same commit.

In summary, there is no decrease in test scenario coverage with the implied test now being explicited checked in spec/models/well_attribute_spec.rb.

test: add example test cases

3b50531

StephenHulme self-assigned this Jan 27, 2025

StephenHulme requested a review from KatyTaylor January 27, 2025 13:23

StephenHulme commented Jan 27, 2025

View reviewed changes

Merge branch 'develop' into y24-382-add-test-cases

eee2f17

KatyTaylor reviewed Jan 29, 2025

View reviewed changes

spec/models/well_spec.rb Outdated Show resolved Hide resolved

KatyTaylor reviewed Jan 29, 2025

View reviewed changes

spec/models/well_spec.rb Outdated Show resolved Hide resolved

StephenHulme commented Jan 29, 2025

View reviewed changes

KatyTaylor reviewed Jan 29, 2025

View reviewed changes

StephenHulme added 15 commits February 4, 2025 11:04

test: add maximum_volume as test parameter

b18f751

Merge branch 'develop' into y24-382-add-test-cases

ee486ce

Merge branch 'develop' into y24-382-add-test-cases

1a801d8

test: move estimated_volume to a separate check

2aa1b7a

test: add assertions for amount final source and destination volumes

292a30b

test: refactor value assignment

708a6e4

test: rename desired to target

090f7f5

test: add final source volumes

a124453

test: add final destination volumes

32c80c3

style: reformat test to improve readbility

15c3b31

test: update final volumes

b139544

style: change test parameter order for reability

df4afdd

refactor: rename _obtained -> _picked

dbaabb4

test: fix broken expectation

f40b62b

style: improve readbility of individual examples

9f37b19

KatyTaylor reviewed Feb 7, 2025

View reviewed changes

KatyTaylor approved these changes Feb 10, 2025

View reviewed changes

test: refactor stock_to_pick and wording

d1f1c5b

StephenHulme merged commit bf13288 into develop Feb 11, 2025
23 checks passed

StephenHulme deleted the y24-382-add-test-cases branch February 11, 2025 08:28

github-actions bot mentioned this pull request Feb 11, 2025

[release] Merge Develop into Master for 14.54.0 #4644

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Y24-382: add example test cases #4641

Y24-382: add example test cases #4641

StephenHulme commented Jan 27, 2025 •

edited

Loading

StephenHulme commented Jan 27, 2025 •

edited

Loading

StephenHulme Jan 27, 2025

KatyTaylor Jan 29, 2025

KatyTaylor Jan 29, 2025

StephenHulme Jan 29, 2025

KatyTaylor Jan 29, 2025

StephenHulme Jan 29, 2025

codecov bot commented Jan 29, 2025 •

edited

Loading

KatyTaylor Jan 29, 2025 •

edited

Loading

StephenHulme Jan 29, 2025

StephenHulme Jan 29, 2025

KatyTaylor Jan 29, 2025

KatyTaylor Feb 7, 2025

StephenHulme Feb 10, 2025

StephenHulme Feb 10, 2025

Y24-382: add example test cases #4641

Y24-382: add example test cases #4641

Conversation

StephenHulme commented Jan 27, 2025 • edited Loading

Changes proposed in this pull request

Instructions for Reviewers

StephenHulme commented Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 29, 2025 • edited Loading

Codecov Report

KatyTaylor Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenHulme commented Jan 27, 2025 •

edited

Loading

StephenHulme commented Jan 27, 2025 •

edited

Loading

codecov bot commented Jan 29, 2025 •

edited

Loading

KatyTaylor Jan 29, 2025 •

edited

Loading