feat(test-data-generation): first pass at generating deck configurations #14962

DerekMaggio · 2024-04-19T17:49:52Z

Overview

With the large code change in #14684 we want to test it as thoroughly as possible. This PR adds generation of test cases with hypothesis for deck configuration

The idea is that hypothesis will generate DeckConfiguration objects that represent what a user defines in their protocol or their deck configuration in the UI. These objects will then end up being used to auto-generate Python protocols to pipe through analysis to exercise our deck configuration validation logic

Test Plan

I went through some of the generated deck configurations to verify they were being created correctly

Changelog

Create datashapes.py
- This defines a simplified deck configuration model and all of its contents
Create helper_strategies.py
- This file provides the building block strategies that are utilized to make a final strategy that makes a DeckConfiguration object
Create final_strategies.py
- This contains the logic for generating the final DeckConfiguration objects

Review requests

Should I add some tests to confirm that DeckConfiguration objects are being generated as expected?

Risk assessment

None.... yet

SyntaxColoring

This all looks reasonable to me so far. Thank you!

test-data-generation/src/test_data_generation/deck_configuration/datashapes.py

SyntaxColoring · 2024-04-23T18:24:21Z

test-data-generation/src/test_data_generation/deck_configuration/assumptions.py

+class EvaluationFunction(typing.Protocol):
+ """A method that validates a deck configuration."""
+
+ def __call__(self, deck: DeckConfiguration) -> bool:
+ """Evaluates the deck configuration and returns True if it is valid."""
+ ...


What is the best way to determine if my created valid and invalid deck configurations meet our internal requirements of being valid and invalid? I am thinking of just starting to throw them at the analysis engine and the deck_configuration endpoint and see what happens.

Our "official" validation lives in robot_server/deck_configuration/validation.py and robot_server/deck_configuration/validation_mapping.py. I don't have an informed opinion about whether it's a good idea to just import that directly, or go through python -m opentrons.cli analyze, or what.

As far as the machine-readable schema is concerned, it's currently pretty weakly-defined. For example, the fixture names are typed as strs instead of enums, and we validate that them "manually," in our application layer, instead of in our model layer. The idea was to avoid duplicating things from the deck definition. A downside to this is that you can't just feed Hypothesis our Pydantic models and have it automatically generate all the valid deck definitions, because it will generate bogus values for those strings.

On the other hand, maybe that's a good thing? Maybe we should be generating a bunch of invalid deck configurations to test with, as long as they don't drown out the valid ones? I'm curious what you think about this, philosophically.

There's also https://pypi.org/project/hypothesis-jsonschema/. I suppose we could use that to generate deck configurations from robot-server's OpenAPI spec. Or add a deck configuration schema to shared-data, and generate it from that. Those options would have the same str vs. enum problem, but we might have more options for dealing with that at the JSON schema level because JSON schemas can reference and include each other.

test-data-generation/src/test_data_generation/deck_configuration/strategies.py

test-data-generation/tests/test_data_generation/deck_configuration/test_deck_configuration.py

CaseyBatten

Overall looks like a good start but I have some questions below regarding a few validations. Also just to clear up some of the theory behind this PR, I think it's important not to mix up the notion of literal "areas" (or addressable areas as it were) on the deck versus the "fixtures" in a deck's configuration file. The deck configuration represents 12 exact physical cutouts in the deck, and the physical hardware mounted within them. The engine interprets that configuration file's information and loads the definitions for the fixtures provided in those 12 cutouts. From those definitions we then find out what "spaces" on the deck are provided by each of our fixtures. A generic singleRightSlot fixture may only provide a "slot" called "D3" on the deck, but a stagingAreaRightSlot fixture would provide a "D3" slot and a "D4" slot. The deck config doesn't see 4 columns, it only sees that some fixture was loaded into cutout D3, what that fixture "creates" is an engine concern.

test-data-generation/src/test_data_generation/deck_configuration/assumptions.py

CaseyBatten

This looks great and the changes so far seem comprehensive. I've got two comments below regarding some possible future behavior to keep in mind, but nothing blocking.

test-data-generation/src/test_data_generation/deck_configuration/deck_evaluation.py

test-data-generation/src/test_data_generation/deck_configuration/datashapes.py

sfoster1

This looks great, love the explicit hypotheses and the approach towards generating the data.

test-data-generation/tests/test_data_generation/deck_configuration/test_deck_configuration.py

test-data-generation/src/test_data_generation/deck_configuration/strategies.py

y3rsh

let me have the protocols

y3rsh · 2024-05-01T18:53:50Z

test-data-generation/tests/test_data_generation/deck_configuration/test_deck_configuration.py

+ suppress_health_check=[HealthCheck.filter_too_much, HealthCheck.too_slow],
+)
+def test_above_below_heater_shaker(deck_config: DeckConfiguration) -> None:
+ """I hypothesize, that any deck configuration with a non-labware slot fixture above or below a heater-shaker is invalid."""


y3rsh · 2024-05-01T18:54:10Z

test-data-generation/tests/test_data_generation/deck_configuration/test_deck_configuration.py

+ suppress_health_check=[HealthCheck.filter_too_much, HealthCheck.too_slow],
+)
+def test_invalid_fixture_in_col_2(deck_config: DeckConfiguration) -> None:
+ """I hypothesize, that any deck configuration that contains at least one, Heater-Shaker, Trash Bin, or Temperature module, in column 2 is invalid."""


❤️ 😊

…ons (#14962) # Overview With the large code change in #14684 we want to test it as thoroughly as possible. This PR adds generation of test cases with [hypothesis](https://hypothesis.readthedocs.io/en/latest/index.html) for deck configuration The idea is that hypothesis will generate DeckConfiguration objects that represent what a user defines in their protocol or their deck configuration in the UI. These objects will then end up being used to auto-generate Python protocols to pipe through analysis to exercise our deck configuration validation logic # Test Plan - I went through some of the generated deck configurations to verify they were being created correctly # Changelog - Create datashapes.py - This defines a simplified deck configuration model and all of its contents - Create helper_strategies.py - This file provides the building block strategies that are utilized to make a final strategy that makes a DeckConfiguration object - Create final_strategies.py - This contains the logic for generating the final DeckConfiguration objects # Review requests - Should I add some tests to confirm that DeckConfiguration objects are being generated as expected? # Risk assessment None.... yet

DerekMaggio changed the base branch from edge to RQA-2612-create-test-data-generation-project April 19, 2024 18:27

DerekMaggio requested review from y3rsh, sfoster1, SyntaxColoring, sanni-t and CaseyBatten April 19, 2024 18:55

DerekMaggio changed the title ~~Deck configuration testing rough draft~~ feat(test-data-generation): first pass at generating deck configurations Apr 19, 2024

DerekMaggio requested a review from a team April 22, 2024 13:21

DerekMaggio marked this pull request as ready for review April 22, 2024 16:36

DerekMaggio requested a review from a team as a code owner April 22, 2024 16:36

DerekMaggio requested a review from b-cooper April 22, 2024 18:20

SyntaxColoring approved these changes Apr 23, 2024

View reviewed changes

CaseyBatten reviewed Apr 23, 2024

View reviewed changes

test-data-generation/src/test_data_generation/deck_configuration/assumptions.py Outdated Show resolved Hide resolved

CaseyBatten reviewed Apr 23, 2024

View reviewed changes

test-data-generation/src/test_data_generation/deck_configuration/assumptions.py Outdated Show resolved Hide resolved

DerekMaggio requested review from CaseyBatten and ahiuchingau April 30, 2024 16:34

DerekMaggio force-pushed the deck-configuration-testing-rough-draft branch from ca287d9 to 008d82d Compare April 30, 2024 16:54

DerekMaggio requested a review from a team as a code owner April 30, 2024 16:54

DerekMaggio force-pushed the deck-configuration-testing-rough-draft branch from 008d82d to 32ee716 Compare April 30, 2024 16:58

DerekMaggio changed the base branch from RQA-2612-create-test-data-generation-project to edge April 30, 2024 17:05

CaseyBatten approved these changes Apr 30, 2024

View reviewed changes

test-data-generation/src/test_data_generation/deck_configuration/deck_evaluation.py Outdated Show resolved Hide resolved

test-data-generation/src/test_data_generation/deck_configuration/datashapes.py Show resolved Hide resolved

DerekMaggio added 9 commits May 1, 2024 10:27

chore: rough draft some test data generation

5c65039

chore: reorganize

991c81c

cleanup

994a6ff

Address some pr comments

460f5f4

chore: assumptions were really really really really slow

87c3253

chore: rework datashapes

8929379

chore: use validity checks and targets

02ce6b1

feat: add Column

60fc394

chore: remove targets

b38c416

DerekMaggio added 5 commits May 1, 2024 10:27

chore: rework filtering

4395313

chore: remove unused stuff

ff78c16

test: add new test

91ec95f

chore: generalize function

1f996ae

chore: separate some logic

949cfd0

DerekMaggio force-pushed the deck-configuration-testing-rough-draft branch from 4e65279 to 949cfd0 Compare May 1, 2024 17:28

sfoster1 approved these changes May 1, 2024

View reviewed changes

test-data-generation/tests/test_data_generation/deck_configuration/test_deck_configuration.py Outdated Show resolved Hide resolved

test-data-generation/src/test_data_generation/deck_configuration/strategies.py Outdated Show resolved Hide resolved

y3rsh approved these changes May 1, 2024

View reviewed changes

DerekMaggio merged commit b15af5e into edge May 1, 2024
29 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(test-data-generation): first pass at generating deck configurations #14962

feat(test-data-generation): first pass at generating deck configurations #14962

DerekMaggio commented Apr 19, 2024 •

edited

Loading

SyntaxColoring left a comment

SyntaxColoring Apr 23, 2024 •

edited

Loading

CaseyBatten left a comment

CaseyBatten left a comment

sfoster1 left a comment

y3rsh left a comment

y3rsh May 1, 2024

y3rsh May 1, 2024

feat(test-data-generation): first pass at generating deck configurations #14962

feat(test-data-generation): first pass at generating deck configurations #14962

Conversation

DerekMaggio commented Apr 19, 2024 • edited Loading

Overview

Test Plan

Changelog

Review requests

Risk assessment

SyntaxColoring left a comment

Choose a reason for hiding this comment

SyntaxColoring Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

CaseyBatten left a comment

Choose a reason for hiding this comment

CaseyBatten left a comment

Choose a reason for hiding this comment

sfoster1 left a comment

Choose a reason for hiding this comment

y3rsh left a comment

Choose a reason for hiding this comment

y3rsh May 1, 2024

Choose a reason for hiding this comment

y3rsh May 1, 2024

Choose a reason for hiding this comment

DerekMaggio commented Apr 19, 2024 •

edited

Loading

SyntaxColoring Apr 23, 2024 •

edited

Loading