TileDB icon indicating copy to clipboard operation
TileDB copied to clipboard

Published 20 hours ago • verified publisher icon TileDB-Inc

Make porting tiledb_unit integration tests to REST CI easier

Open ypatia opened this issue 1 year ago • 2 comments

This PR groups some improvements on testing infra for REST tests:

  • Introduce class VFSTestSetup as an easy way to set up an integration test in tiledb_unit for all VFSs and REST.
  • Adapts existing REST-CI tests and adds some new tests to use that class.
  • Removes --enable-rest-tests bootstrap and corresponding TILEDB_TESTS_ENABLE_REST options.
  • Adds tags [rest] for tiledb_unit tests to run on REST CI, [non-rest] for tests that don't apply to REST, and [rest-fails] to tests that should be run on REST but fail because of issues logged here: https://app.shortcut.com/tiledb-inc/story/40489/issues-found-while-running-tiledb-unit-core-tests-against-rest-cloud-server

Note: REST-CI runner needs to be adapted after this is merged, but it's in a separate repo if I understand well. Also an extra runner will be added to test Query v3 that will be setting use_refactored_array_open_and_query_submit config to true by default and run all [rest] tests.

TYPE: IMPROVEMENT DESC: Make porting tiledb_unit integration tests to REST CI easier

ypatia avatar Jan 29 '24 15:01 ypatia

This pull request has been linked to Shortcut Story #37953: Augment current REST-CI tests around array open and running queries..

shortcut-integration[bot] avatar Jan 29 '24 15:01 shortcut-integration[bot]

Please ask me to review this when it is ready.

teo-tsirpanis avatar Feb 05 '24 17:02 teo-tsirpanis

LGTM once CI is green, nice work 👍 REST CI will fail since this PR removes bootstrap flags being used in those workflows.

If you want to test this PR against TileDB-REST-CI we can update the ref parameter in ci-rest.yml to point to a different branch on TileDB-REST-CI. We would definitely want to revert that change before merging though. If you would like to test this way and run into issues there or want me to spin up a branch on TileDB-REST-CI for testing LMK, happy to help.

Thanks for the idea and the pointers! I did that (and reverted) against my Tiledb-REST-CI branch: https://github.com/TileDB-Inc/TileDB-REST-CI/pull/17 And it passes: https://github.com/TileDB-Inc/TileDB-REST-CI/actions/runs/7987300561/job/21809494707 . So, as soon as CI of the current PR passes we are good to merge both. FYI: @KiterLuc

ypatia avatar Feb 21 '24 10:02 ypatia