Skip to content

Conversation

@yamini-labelbox
Copy link
Contributor

@yamini-labelbox yamini-labelbox commented Aug 1, 2023

https://labelbox.atlassian.net/browse/AL-5942

  • reorganize exports test cases
  • add more coverage in terms of validating individual exports attributes

tests/integration/annotation_import/test_data_types.py::test_import_data_types_v2 still has the exports v2 tests for all the datatypes. This test cases does testing for both import and export pipelines. So I let it be here for now. Although it is ideal to have a similar test case in the exports suite. But in view of not duplicating the test cases (which increases the run time), i let it be here. This is the only exports v2 related test case that is not inside the exports_v2 suite.

  • there are a few new ones added in the past week. i will have another PR to move those around once i merge in this one

@yamini-labelbox yamini-labelbox marked this pull request as ready for review August 2, 2023 00:39
@yamini-labelbox yamini-labelbox changed the title exports v2 integtests [AL-5942] Exports v2 integration tests Aug 2, 2023
@yamini-labelbox yamini-labelbox force-pushed the yk/InTests branch 6 times, most recently from 1749dc8 to 58e277e Compare August 2, 2023 20:34
@yamini-labelbox yamini-labelbox requested review from a team, apollonin and vbrodsky August 7, 2023 22:03
import pytest


@pytest.mark.parametrize('data_rows', [5], indirect=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this have to be 5 data rows? Can we reduce to 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can. Does it impact the the test run time by a large extent? (curious)
I am dropping it to 2 anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

well as I am working right now to optimize fixtures, it looks like the biggest bottleneck is creation of data rows, because this has to go to es and come back
I can not say that 2 vs 5 will make a world of difference, but generally I think we never need more than 2 data rows in our integration tests to represent the concept of 'many'.

]) == set(data_row_ids)


@pytest.mark.parametrize('data_rows', [5], indirect=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, why 5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed it to 3. This test needs a min of 3 datarows.

@yamini-labelbox yamini-labelbox merged commit a85420c into develop Aug 11, 2023
@yamini-labelbox yamini-labelbox deleted the yk/InTests branch August 11, 2023 17:36
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