openmicroscopy icon indicating copy to clipboard operation
openmicroscopy copied to clipboard

Extend test coverage for tables slice, read and readCoordinates API

Open sbesson opened this issue 1 year ago • 5 comments

See https://github.com/ome/openmicroscopy/pull/6411#issuecomment-2346161017 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.

sbesson avatar Sep 18 '24 09:09 sbesson

See https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/173/testReport/OmeroPy.test.integration.tablestest.test_service/TestTables/

sbesson avatar Sep 19 '24 07:09 sbesson

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 https://github.com/ome/omero-py/pull/430.

sbesson avatar Sep 19 '24 15:09 sbesson

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 avatar Sep 25 '24 08:09 sbesson

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.

https://github.com/ome/omero-py/pull/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 avatar Sep 26 '24 10:09 sbesson

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.

sbesson avatar Sep 26 '24 15:09 sbesson