Skip to content

test(robot-server): Test compatibility with databases created in v6.0#11448

Merged
SyntaxColoring merged 14 commits into
edgefrom
test_persistence_compatibility
Sep 20, 2022
Merged

test(robot-server): Test compatibility with databases created in v6.0#11448
SyntaxColoring merged 14 commits into
edgefrom
test_persistence_compatibility

Conversation

@SyntaxColoring

@SyntaxColoring SyntaxColoring commented Sep 8, 2022

Copy link
Copy Markdown
Contributor

Overview

This adds a basic test for persistence backwards compatibility, which includes backwards compatibility in our SQL database. The test makes sure that the current robot-server code can read some resources that were created by a v6.0.1 robot-server.

Closes RSS-102.

This is a humongous diff—sorry! Most of it is opaque fixture data. The actual code changes are fairly small.

Changelog

  • Copy a pre-populated persistence directory into the repository, intended to be used as an opaque test fixture. I created it by manually issuing HTTP requests to a v6.0.1 dev server and saving its persistence directory. See the added readme for details.

  • Add a test that runs a dev server underpinned by that persistence directory, issues a bunch of HTTP GET requests to it, and makes sure none of the requests come back with an obvious error.

    For simplicity, we only check the responses in a very shallow way. Basically, we just check that the HTTP status is successful, and that the response data isn't empty. I believe this is sufficient to catch the bugs that we've been prone to so far.

Review requests

  • Will these tests give us sufficient confidence to rework our persistence code according to the plan in RSS-98?
  • Try making a change in robot-server, api, or shared-data that would break backwards compatibility. For example, try making an Optional field in a persisted Pydantic model non-Optional. Then, run make -C robot-server test—does it catch the problem?
  • Do you have any ideas for future improvements? If so, please record them in RSS-100.

Risk assessment

No risk to production code. Changes are only to tests.

"params": {
"pipetteId": "50d23e00-0042-11ec-8258-f7ffdf5ad45a",
"mount": "right"
"mount": "left"

@SyntaxColoring SyntaxColoring Sep 8, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This looked like a mistake in the fixture. Both pipettes were being loaded on the right mount, which caused simulation and run problems. Elsewhere in this file, within designerApplication, this pipette is said to be on the left mount, so I changed this to match.

@codecov

codecov Bot commented Sep 8, 2022

Copy link
Copy Markdown

Codecov Report

Merging #11448 (569754e) into edge (715b2a0) will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #11448      +/-   ##
==========================================
- Coverage   74.31%   74.22%   -0.09%     
==========================================
  Files        2072     2083      +11     
  Lines       57377    57468      +91     
  Branches     5539     5550      +11     
==========================================
+ Hits        42638    42655      +17     
- Misses      13482    13555      +73     
- Partials     1257     1258       +1     
Flag Coverage Δ
app 74.20% <ø> (-0.44%) ⬇️
protocol-designer 45.81% <ø> (ø)
shared-data 86.44% <ø> (ø)
step-generation 88.39% <ø> (ø)

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

Impacted Files Coverage Δ
app/src/organisms/ChangePipette/Instructions.tsx 76.92% <0.00%> (-0.86%) ⬇️
app/src/App/index.tsx 100.00% <0.00%> (ø)
app/src/molecules/Modal/index.tsx 100.00% <0.00%> (ø)
app/src/atoms/StatusLabel/index.tsx 100.00% <0.00%> (ø)
.../python/opentrons_shared_data/pipette/dev_types.py 100.00% <0.00%> (ø)
app/src/atoms/buttons/index.tsx
app/src/atoms/Toast/Toast.stories.tsx 0.00% <0.00%> (ø)
app/src/atoms/buttons/SubmitPrimaryButton.tsx 100.00% <0.00%> (ø)
app/src/organisms/CalibrationStatusCard/index.tsx 66.66% <0.00%> (ø)
...p/src/pages/Devices/CalibrationDashboard/index.tsx 80.00% <0.00%> (ø)
... and 10 more

Comment thread robot-server/.flake8
@SyntaxColoring SyntaxColoring marked this pull request as ready for review September 9, 2022 21:05
@SyntaxColoring SyntaxColoring requested review from a team as code owners September 9, 2022 21:05
Comment thread robot-server/tests/integration/http_api/persistence/test_compatibility.py Outdated

@mcous mcous left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few little thoughts but this seems like a pretty vast improvement in our regression detection abilities for these APIs

Comment thread robot-server/mypy.ini

@TamarZanzouri TamarZanzouri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome work Max! Thank you!

@SyntaxColoring SyntaxColoring merged commit 7273aa6 into edge Sep 20, 2022
@SyntaxColoring SyntaxColoring deleted the test_persistence_compatibility branch September 20, 2022 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants