Skip to content

add pragma - utils_test#5799

Open
scharlottej13 wants to merge 5 commits intodask:mainfrom
scharlottej13:improve-coverage-utils_test
Open

add pragma - utils_test#5799
scharlottej13 wants to merge 5 commits intodask:mainfrom
scharlottej13:improve-coverage-utils_test

Conversation

@scharlottej13
Copy link
Copy Markdown
Contributor

Pragma no cover

  • We have cases with relatively long switch-like statements ending in a final exception, e.g. ValueError "unknown value"
  • Conditional imports may need be covered with pragma statements and/or a dedicated job

Moved this into a separate PR from #5749. This one was a bit tricky, when unsure, I added pragma no cover where the only reference was in a test that is already ignored in .coveragerc; if the function was called elsewhere in utils_test.py, I opted not to exclude. For some functions, I had trouble finding a reference (e.g deep, slowdouble).

@GPUtester
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

@scharlottej13 scharlottej13 mentioned this pull request Feb 12, 2022
1 task
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 12, 2022

Unit Test Results

       12 files  ±  0         12 suites  ±0   7h 4m 34s ⏱️ + 4m 24s
  2 602 tests +  1    2 516 ✔️ +    5       84 💤  -     5  2 +1 
15 542 runs   - 16  14 471 ✔️  - 190  1 069 💤 +173  2 +1 

For more details on these failures, see this check.

Results for commit b04d5c7. ± Comparison against base commit 6a17957.

♻️ This comment has been updated with latest results.

Comment thread distributed/utils_test.py Outdated
Comment on lines 101 to 105

@pytest.fixture(scope="session")
@pytest.fixture(scope="session") # pragma: no cover
def valid_python_script(tmpdir_factory):
local_file = tmpdir_factory.mktemp("data").join("file.py")
local_file.write("print('hello world!')")
return local_file
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.

This appears to be ancient and we should remove this function. Using the github search, this only pops up in distributed forks and was used in tests that have been removed two years ago in #3280

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.

This is ancient stuff :)

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.

FYI

The github search shows a test named test_dask_submit_cli_writes_result_to_stdout. Finding commits in the git history which touch this test is easy with git log, e.g.

git log -Stest_dask_submit_cli_writes_result_to_stdout shows all commits touching that test

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.

that is very helpful, thank you!

Comment thread distributed/utils_test.py Outdated

@pytest.fixture(scope="session")
@pytest.fixture(scope="session") # pragma: no cover
def client_contract_script(tmpdir_factory):
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.

same here, please remove

Comment thread distributed/utils_test.py Outdated

@pytest.fixture(scope="session")
@pytest.fixture(scope="session") # pragma: no cover
def invalid_python_script(tmpdir_factory):
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.

same here, please remove

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