Skip to content

Fix C&P error in test_toy_cuda_sanity_check and simplify#5183

Open
Flamefire wants to merge 2 commits into
easybuilders:developfrom
Flamefire:fix-cuda-test
Open

Fix C&P error in test_toy_cuda_sanity_check and simplify#5183
Flamefire wants to merge 2 commits into
easybuilders:developfrom
Flamefire:fix-cuda-test

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

The major issue was in assert_cuda_report which wrongly was calling assert_regex(num_checked_str, outtxt, stdout).
But the parameter to the function is log not outtxt

This went undetected because we do not enable the Flake8 check for unused variables/parameters.
I'd argue we should, even if that means a bit initial refactoring: Most uses should be ok, so we simply silence the warning by using _ as the name or prefix the variable with an underscore.

While doing that I noticed that there are a lot of assertTrue checks when testing for a regex. In #5137 I replaced many of those already by the better assertRegex which avoids us having to specify the message.
This removes almost half of the lines of the test: 1. Compile regex, 2. Assemble message, 3. do the check are replaced by just doing the check.

There were also cases where the regex was wrong, e.g. r"Missing compute capabilities: 8.0."
The dot would need to be escaped, but the better check is assertIn, we don't need a regex here.

Use `assertRegex` or `assertIn` and remove message variables
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.

1 participant