Skip to content

Use assert(Not)Regex instead of assertTrue or assertFalse in tests#5188

Open
Flamefire wants to merge 3 commits into
easybuilders:developfrom
Flamefire:assertregex
Open

Use assert(Not)Regex instead of assertTrue or assertFalse in tests#5188
Flamefire wants to merge 3 commits into
easybuilders:developfrom
Flamefire:assertregex

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

@Flamefire Flamefire commented May 5, 2026

This makes the tests shorter and hence easier to read, avoids
re.compile calls and manual failure message composing.
In some cases assert_multi_regex could be used to avoid the explicit loops.

While doing that I put the re.compile into the assertion instead of assigning to a variable and remove the call when there are no flags required as assertRegex accepts strings or compiled expressions

Examples:

-        regex = re.compile(os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-[0-9]{8}\.[0-9]{6}\.log$'))
-        self.assertTrue(regex.match(res), "Pattern '%s' matches '%s'" % (regex.pattern, res))
+        self.assertRegex(res, os.path.join(tmpdir, r'easybuild-foo-1\.2\.3-[0-9]{8}\.[0-9]{6}\.log$'))

-        regex = re.compile(r"^eb toy-0.0.eb --robot --debug -l", re.M)
-        self.assertTrue(regex.search(txt), "Pattern '%s' should be found in: %s" % (regex.pattern, txt))
+        self.assertRegex(txt, re.compile(r"^eb toy-0.0.eb --robot --debug -l", re.M))

-            fail_msg = "Pattern '%s' should be found in: %s" % (guarded_load_regex.pattern, modtxt)
-            self.assertTrue(guarded_load_regex.search(modtxt), fail_msg)
-            fail_msg = "Pattern '%s' should not be found in: %s" % (recursive_unload_regex.pattern, modtxt)
-            self.assertFalse(recursive_unload_regex.search(modtxt), fail_msg)
+            self.assertRegex(modtxt, guarded_load_regex)
+            self.assertNotRegex(modtxt, recursive_unload_regex)

Continuation of #4205 , #4170, #5134 to remove usage of assertTrue & assertFalse as for almost everything there is a better alternative

Also regex assertions are over-used. In some places assertIn or even assertEqual could be used which is clearer and faster.
However the replacements done here were mostly regex-based with lot's of manual inspection to find good patterns, and finding those where assertIn would work is even more difficult.

Requires:

Flamefire added 2 commits May 5, 2026 10:27
When running `bash -i` with `-c` it might have unintended side effects,
like modifying the parent processes state.
This manifests e.g. in the tests:
`python -m test.framework.suite test_run_shell_cmd_basic` causes the
process to go to background and stall until typing `fg` on the 2nd
invocation of `run_shell_cmd` with `cmd.sh -c ...`

Using `BASH_ENV` for the non-interactive use avoids this while still
sourcing the env file.
To detect that the presence of any argument to `cmd.sh` is used as for
interactive usage none would be present.
Otherwise the space won't be printed when it is empty and the regex fails.
This makes the tests shorter and hence easier to read, avoids
`re.compile` calls and manual failure message composing.
In some cases `assert_multi_regex` could be used to avoid the explicit loops.
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