Skip to content

Improve datalad.support.tests.test_parallel.py::test_stalling - #7119

Merged
yarikoptic merged 2 commits into
datalad:maintfrom
christian-monch:imp-test-stalling
Oct 26, 2022
Merged

Improve datalad.support.tests.test_parallel.py::test_stalling#7119
yarikoptic merged 2 commits into
datalad:maintfrom
christian-monch:imp-test-stalling

Conversation

@christian-monch

@christian-monch christian-monch commented Oct 25, 2022

Copy link
Copy Markdown
Contributor

This PR extends the time after which we believe to witness a stalling thread to at least 5 seconds. This time-span will only be exhausted if the test is failing, i.e. an executing worker is stalling (or very very slow). The duration of a passing test is usually in the range of a few dozen milliseconds.

The commit adds output processing in order to actually start of few runner-related threads and performs thread-based asynchronous data processing.

The large time limit will hopefully reduce the number of false positives greatly, though it cannot guarantee the absence of false positives, it just makes them unlikely, i.e. less than 1 false positive per 10e5 executions.

This commit extends the time after which we believe
to see a stalling thread to at least 5 seconds. This
time will only be exhausted, if the thread is stalling,
the duration of a passing test is usually in the range
of a few dozen milliseconds.

The commit adds output processing in order to actually
start of few runner-related threads and perform
thread-based asynchronous data processing.

The huge time limit will hopefully reduce the number
of false positives greatly, though is cannot *guarantee*
the absence of false positives, it makes them unlikely,
i.e. less than 1 false positive per 10e5 executions.
@christian-monch christian-monch added the semver-tests Changes only affect tests, no impact on version label Oct 25, 2022
@qlty-cloud-legacy

Copy link
Copy Markdown

Code Climate has analyzed commit eded6ae and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 5

View more on Code Climate.

@christian-monch
christian-monch changed the base branch from master to maint October 25, 2022 20:13
@codecov

codecov Bot commented Oct 25, 2022

Copy link
Copy Markdown

Codecov Report

Base: 89.37% // Head: 90.93% // Increases project coverage by +1.55% 🎉

Coverage data is based on head (7a61bbe) compared to base (dfc97e7).
Patch coverage: 87.50% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##            maint    #7119      +/-   ##
==========================================
+ Coverage   89.37%   90.93%   +1.55%     
==========================================
  Files         355      355              
  Lines       46497    46498       +1     
  Branches     6327     6327              
==========================================
+ Hits        41558    42283     +725     
+ Misses       4924     4200     -724     
  Partials       15       15              
Impacted Files Coverage Δ
datalad/support/tests/test_parallel.py 95.23% <87.50%> (+0.03%) ⬆️
datalad/_version.py 45.68% <0.00%> (ø)
datalad/__init__.py 98.00% <0.00%> (+16.00%) ⬆️
datalad/tests/utils.py 56.96% <0.00%> (+35.36%) ⬆️
datalad/tests/test_tests_utils.py 92.34% <0.00%> (+92.34%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yarikoptic

Copy link
Copy Markdown
Member

Thank you @christian-monch !

@yarikoptic
yarikoptic merged commit b33dcb3 into datalad:maint Oct 26, 2022
@yarikoptic-gitmate

Copy link
Copy Markdown
Collaborator

PR released in 0.17.9

@datalad datalad deleted a comment from yarikoptic-gitmate Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-tests Changes only affect tests, no impact on version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants