Improve tests code even more

[perillo]

Additional improvements to kcov test suite.

This PR changes a lot of code! I used sed to make the change automatic.

CHANGES

• [x] tests: use subprocess.run to run child process

  The current, old API, is not only inconvenient (using a thread and a timer to implement timeouts) but it is also incorrect since the child process is not waited.

  Use subprocess.run for both doShell and do. Note that for the do method, this is a behavior change, since a child process is always terminated by kill.

  Add a default timeout, set to 10 min.

  After this change the test system_mode_can_record_and_report_binary fails. This will be addressed in the next commit.

• [x] tests: make shutil.rmtree more robust

  In some rare case shutil.rmtree from KcovTestCase.tearDown may fails, in case a child process continue to write additionally files.

  I can reproduce the behavior consistently by setting default_timeout to 1 second.

  Update the tearDown method to retry rmtree in case of a ENOTEMPTY error, waiting 5 seconds to ensure that the offending child process terminates.

• [x] tests: improve custom messages in a test case

  A few tests use print to show a custom message, but the problem is that this will break the test status line.

  Add the KcovTestCase.write_message method, so that the message is showed correctly in the test status line.

• [x] tests: rename the parse_cobertura module to cobertura

  The old name is long and not concise.

  As an example: parse_cobertura.parseFile(path) parse_cobertura.parse(data)

• [x] tests: remove unnecessary creation of outdir

  The system_mode_can_record_and_report_binary test tries to create the outdir directory, but it is already created by TestCase.setUp.

• [x] tests: use consistent naming convention in test cases

  All the test case names use the snake case convention, except the basic module that uses the camel case convention.

  Make naming convention consistent.

• [x] tests: make configuration values KcovTestCase members

  Update the testcase.KcovTestCase.setUp method to add the configuration values as class members instead of globals.

  These global variables are still needed, but are hidden from the test modules. Remove the global kcov_system_daemon variable, since it is no longer necessary.

  Update all the test cases and reformat the code with ruff format.

• [x] tests: add testbase.KcovTestCase.doCmd method

  The new function is meant to be used when running a child process that does not run kcov, both directly or indirectly. The method call usually returns noKcovRv, instead of rv.

  The reason for this change is to make these calls easier to detect, also avoiding having to set the kcovKcov parameter to False.

• [x] tests: make calls to KcovTestCase.do more strict

  Update the KcovTestCase.do method signature, so that kcovKcov is both a positional or keyword parameter, and the timeout is a keyword only parameter.

  I was planning to make kcovKcov a positional only parameter, but the bash_linux_only.bash_exit_before_child test used it as a keyword parameter.

  This change codifies the current usage.

TODO

Additionally tests that should use doCmd:

• system_mode.system_mode_can_start_and_stop_daemon
• system_mode.system_mode_can_record_and_report_binary

Possible BUG(?)

• basic.lookup_binary_in_path should set kcovKcov to False

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.68%. Comparing base (c3b0b2a) to head (9794dd0).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #432 +/- ## ========================================== - Coverage 65.70% 65.68% -0.03% ========================================== Files 58 58 Lines 4514 4514 Branches 4171 4171 ========================================== - Hits 2966 2965 -1 - Misses 1548 1549 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[perillo]

As I suspected system_mode_can_record_and_report_binary did not failed on Github CI. Probably because the tests are run as root, while on my PC the tests are run as a normal user.

The strange thing is that system_mode_can_record_and_report_binary is incorrect, since it starts the daemon process but does not stop it.

[SimonKagstrom]

Thanks!

As for system-mode, it has really never been properly working anyway. It's sort of meant to be run as root, since the purpose is to collect coverage from all binaries in a system, as a part of the startup of an embedded system. However, it really was never working outside of a proof-of-concept stage, so I think we can simply disable the system mode tests for now.

[perillo]

Thanks for the explanation.

However it is true that before the change introducing subprocess.run, the test worked until the skipTest with normal privilege.

Do you have a preferred channel for communication, like email, forum or github issue? For the following commits I may need your help for some clarification.

[SimonKagstrom]

I'm fine with pr communication!

What I mean is mainly that we can just remove the system mode tests. Maybe also the build of the binary, since it's not really working other than as a toy.

[SimonKagstrom]

Good stuff! The tests feel much more proper now, and not like the plain hacks that they were before :-)

Anyway, I've looked through your changes, and I think they look good! Tell me when you're ready and I'll merge them!

[perillo]

Good stuff! The tests feel much more proper now, and not like the plain hacks that they were before :-)

Anyway, I've looked through your changes, and I think they look good! Tell me when you're ready and I'll merge them!

The PR is ready for merge.

However I would like to update the basic.lookup_binary_in_path test to use doCmd in

noKcovRv, o = self.do(self.sources + "/tests/python/main 5")

I avoided the change since I was not sure of it was a oversight to not set kcovKcov to False.

Additionally, I found that in this pattern

noKcovRv, o = self.doCmd(self.testbuild + "/executable")
rv, o = self.do(self.kcov + " " + self.outbase + "/kcov " + self.testbuild + "/executable", False)

a few tests don't set kcovKcov to True. There are no comments, so I'm not sure if this was the correct way or an oversight.

I checked the code using

> grep -A15 noKcovRv tests/tools/*.py`

[perillo]

I forgot to add a commit.

Fixed a typo in the commit message.

[SimonKagstrom]

Big thanks!

In general, if I remember correctly, the kcovKcov stuff was to avoid running kcov on itself when collecting data for compiled code, where it's not possible to trace kcov itself since it would mean recursive ptrace, which is illegal. Thus, coverage data on kcov itself can be collected when covering bash and python, but not for compiled code.