Skip to content

SOLR-16092 Do not fail test on slow CI servers#738

Merged
madrob merged 2 commits into
apache:mainfrom
madrob:SOLR-16092
Mar 11, 2022
Merged

SOLR-16092 Do not fail test on slow CI servers#738
madrob merged 2 commits into
apache:mainfrom
madrob:SOLR-16092

Conversation

@madrob
Copy link
Copy Markdown
Contributor

@madrob madrob commented Mar 10, 2022

@madrob madrob requested a review from murblanc March 10, 2022 01:53
Comment on lines -54 to -58
testParallelCollectionAPICalls();
testTaskExclusivity();
testDeduplicationOfSubmittedTasks();
testLongAndShortRunningParallelApiCalls();
testFillWorkQueue();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Turning the five private methods into top-level tests means they could run in different orders, so more realistic test coverage there as a side effect, nice.

From a quick read of the tests, they don't look like one could cause side effects that would change the baseline for the next tests. So changing order should be fine.

Am less sure though on whether or not the tests could run in parallel now then and if then they could interact with each other? The ones which create collections, the collection names are specific to each test, so no problem there at least.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MAX_PARALLEL_TASKS in testFillWorkQueue() might have a cross test impact? When the queue is full, it's full.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess it might just delay the other tests if running concurrently. Let's leave it like this and see if new problems appear?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think that we run tests from the same class concurrently in the same JVM, but that also seems like an implementation detail that we shouldn't rely on. Will watch for breakage.

@madrob madrob merged commit a63a6fb into apache:main Mar 11, 2022
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