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

Skip sync tests in Cosmos #33756

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Skip sync tests in Cosmos #33756

wants to merge 1 commit into from

Conversation

roji
Copy link
Member

@roji roji commented May 19, 2024

#33386 removed sync I/O support from Cosmos; the approach for the tests was to override e.g. all query tests, and introduce a special "NoSyncTest" wrapper to catch the special exception thrown when the test runs in the sync variant, and introduces is (async) checks in many tests. This forces us to deal with the sync case in each and every test.

This draft PR proposes to instead handle this via a simple [SkipSyncTests] attribute, which is used at the assembly level; if the test method being executed has a bool async parameter, and that parameter has a false argument, the test is skipped. We can then add a few targeted tests for the sync case which check that the appropriate exception is thrown.

This is a partial implementation, for now only for query and with some failures; we can complete it if we agree this is we want.

@roji roji requested a review from ajcvickers May 19, 2024 12:18
@ajcvickers
Copy link
Member

Seems like a good approach to me.

@roji roji force-pushed the CosmosQueryTest branch 2 times, most recently from 489f3db to b2a0e83 Compare May 30, 2024 17:29
@roji roji marked this pull request as ready for review May 30, 2024 17:43
@roji
Copy link
Member Author

roji commented May 31, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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.

None yet

2 participants