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

Extend test coverage for tables slice, read and readCoordinates API #6412

Merged
merged 7 commits into from
Nov 20, 2024

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Sep 18, 2024

See #6411 (comment) for the initial motivation.

The contract around the handling of empty inputs in the omero.tables.slice API is currently solely documented in the slice
generated API documentation - see https://docs.openmicroscopy.org/omero-blitz/5.7.3/slice2html/omero/grid/Table.html#slice.
This PR expands the table integration tests to check different scenarios including:

  • empty input in tables.slice
  • invalid inputs for the tables.slice, tables.read and tables.readCoordinates
  • return order for tables.slice

The new tests are expected to pass with the current version of OMERO.py. As part of this, I have identified an issue when start/stop are outside the rows range in table.read() which I will address separetely.

- add test case for the special handling of empty inputs in table.slice
- add various test cases for invalid inputs in the table reading API
@sbesson
Copy link
Member Author

sbesson commented Sep 19, 2024

Added a few tests around the the expectations of the table.read() and table.readCoordinates API. This highlighted a bug in OMERO.py when using some start/end values and a corresponding fix has been opened as ome/omero-py#430.

@sbesson
Copy link
Member Author

sbesson commented Sep 25, 2024

Added a few tests to cover more scenarios around table.addData and table.updateData.

  • I split the table.read tests into 3 categories (start=end, start<end and start / end outside the [0,nrows-1] range). I marked the start=end tests and start/end out of range scenarios as broken as additional fixes are required and some of the current behavior is at odds with the note in https://omero.readthedocs.io/en/stable/developers/Tables.html#omero.grid.Table.read
  • testCannotUpdateOutOfRange currently fails with an InternalException as there is no out of range check in the OMERO.py tables implementation. I will open an OMERO.py PR

@sbesson
Copy link
Member Author

sbesson commented Sep 26, 2024

As expected, testCannotUpdateOutOfRange introduced in 2744425 failed in https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/179/testReport/OmeroPy.test.integration.tablestest.test_service/TestTables/testCannotUpdateOutOfRange/ as the server currently raises an InternalException.

ome/omero-py#431 should modify the implementation so that updating rows out of range now raises a proper ApiUsageException. With both PRs, I would expect all OMERO.table tests to turn green in the next round of CI integration tests and to be ready for another round of review

@sbesson
Copy link
Member Author

sbesson commented Sep 26, 2024

See https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/180/testReport/OmeroPy.test.integration.tablestest.test_service/ for the latest passing CI build including these new integration tests and the associated OMERO.py fix.

@jburel jburel merged commit b317570 into ome:develop Nov 20, 2024
4 checks passed
@sbesson sbesson deleted the table_more_tests branch November 20, 2024 11:44
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