use better test assertions by replacing use of assertFalse/assertTrue

[Flamefire]

Replace self.assertTrue(False, ... and similar by appropriate assert functions. This avoids redundancy and potential mistakes and improves error reporting.

I checked that all functions are available in Python 2.7 and 3.1+

Especially the assertTrue(False were annoying as the error message started with "Assertion failed: False is not True"

Most of the assert[Not]In replace the need to duplicate the arguments in the message which sometimes got forgotten.

Mostly done by a RegEx search&replace so should be save.

I also fixed a bug in the test:

mods = ['GCC/6.4.0-2.28', 'hwloc/1.11.8-GCC-6.4.0-2.28', 'OpenMPI/2.1.2-GCC-6.4.0-2.28']
self.assertTrue([m['mod_name'] for m in self.modtool.list()], mods)

Here assertEqual was intended as shown by the 2nd parameter. Another reason why we should try to avoid the generic assertTrue/False tests

[boegel]

@Flamefire Thanks a lot for the effort here, makes a lot of sense.

Before I start wading through this to make sure we're not accidentally introducing a check that always passes, I think it makes sense to prevent that we re-introduce the use of self.assertFalse or self.assertTrue.

For this, we should add a test in test/framework/general.py that searches for the use of these, and fails if it finds any?
That can be done by iterating over all test/framework/*.py files, and using "\s+self\.assert(False|True)" as regex, I think...

edit: nevermind, not all assertFalse/assertTrue are removed here, so that won't work...

[Flamefire]

Before I start wading through this to make sure we're not accidentally introducing a check that always passes, I think it makes sense to prevent that we re-introduce the use of self.assertFalse or self.assertTrue.

There are cases where those make sense. E.g. one compares the GITHUB_TOKEN and purposely is not using assertEqual to avoid it being printed. So a check like that is not possible.

Of course we could use # noqa style comments to annotate such lines. But I'd say that's a bit to much...

Other solution: Switch to pytest and use assert foo == bar instead. No need to worry about the test function to use ;-) But that would be a larger effort.

[boegel]

There's definitely advantages in a switch to pytest, but yes, that would be a big effort.
Although that's probably something that could be done gradually...

It would be nice if we could outline some "best practices" in the EasyBuild documentation w.r.t. writing tests, to add to https://docs.easybuild.io/en/latest/Unit-tests.html

[boegel]

lgtm

[boegel]

Spent some time scrolling through this, didn't spot anything suspicious that would require changes, so good to go.

Thanks a lot for the effort here @Flamefire!