Extend test coverage for tables slice, read and readCoordinates API - Githubissues

ome / openmicroscopy

OME (Open Microscopy Environment) develops open-source software and data format standards for the storage and manipulation of biological light microscopy data. A joint project between universities, research establishments and industry in Europe and the USA, OME has over 20 active researchers with strong links to the microscopy community. Funded by private and public research grants, OME has been a major force on the international microscopy stage since 2000.

https://www.openmicroscopy.org/omero

GNU General Public License v2.0

200 stars 102 forks source link

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

Open sbesson opened 1 month ago

sbesson commented 1 month ago

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 commented 1 month ago

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

sbesson commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.