Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend current GitHub Actions CI (was: No CI running the test suite?) #338

Open
hartwork opened this issue Dec 16, 2024 · 19 comments
Open

Comments

@hartwork
Copy link
Contributor

hartwork commented Dec 16, 2024

Hi!

I noticed the test target in the Makefile but did not find any CI that would run the tests for pushes to Git or new pull requests. Is there a CI that I did not find? A CI would protect against changes that break existing tests without noticing.

PS: If you welcome help with a simple CI based on GitHub Actions targeting Linux, please let me know.

Best, Sebastian

@henry2cox
Copy link
Collaborator

Absolutely: if you would like to pitch in to create some automation - that would be great.
(There are a few jenkins jobs - but not publicly visible ones....that helps me/my team, but nobody else.)

If you do want to create a job or set of jobs, here are a few considerations:

  • probably want to test with gcc version: earlier than 9, 9.X, and latest (they use slightly different gcov interfaces)
  • testing with LLVM as well would be great, but would require some changes to the test infrastructure (see hacks in ../tests/lcov/mcdc).
  • there are versioning, annotatiion, and diff scripts for both git and Perforce. It would be good to run tests with both (...but you can only do that if you have a p4 license...so may not be possible.)
  • there is an environment variable LCOV_FORCE_PARALLEL=1 that does what it says on the tin - and will cause some (but not all) of the parallel processing code to be executed (even though the testcase sizes are too small for parallelism to help).
    That is: it is worth to run the tests with at least one compiler, with and without the flag.
    To exercise all of the parallel processing code requires some examples to time out or space out, and to require retries, etc. This isn't easy to automate.
  • there is a Makefile flag COVERAGE=1 which will collect coverage data for the perl and python code - as well as generating a report at the end. (You really want to run multiple compilers, various settings - then aggregate the data into a report.)
  • A (much) better methodology would include that (aggregated) coverage data in the release such that the CI job could access the previous data and the label - such that the job would generate a differential report to show changes.
    Desirable - but probably not possible/not easy - if the job would fail unless some coverage criteria was met.

If you - or anyone else - feels like adding tests to improve our quality: that would be great.

If you - or anyone else - would like to rewrite/refactor the regression infrastructure: that would also be fantastic.
The current mechanism works - but is relatively brittle and inflexible.

@hartwork
Copy link
Contributor Author

@henry2cox let's start with something simple, get that in mergable state and then grow further from there. Pull request #349 is maybe 90% of what we need to get started. I hope you don't feel overwhelmed by the related 3 new issues and 3 side pull requests I had to open: it was the minimum I could do. No rush, and review welcome 🙏

@hartwork hartwork changed the title No CI running the test suite? Extend current GitHub Actions CI (was: No CI running the test suite?) Dec 18, 2024
@henry2cox
Copy link
Collaborator

How hard is it to artifact/publish (whatever term you like) the 'test.log' file file generated by the make check target?
Path appears to be /home/runner/work/lcov/lcov/tests/test.log (though not sure how the parent directory is chosen).

For (slightly) later: how hard is it to add another target, to build a coverage report (HTML tree) - and then publish that too?

@hartwork
Copy link
Contributor Author

How hard is it to artifact/publish (whatever term you like) the 'test.log' file file generated by the make check target? Path appears to be /home/runner/work/lcov/lcov/tests/test.log (though not sure how the parent directory is chosen).

@henry2cox not hard, let me offer a pull request for that. Starting after this reply…

For (slightly) later: how hard is it to add another target, to build a coverage report (HTML tree) - and then publish that too?

I would say "probably not hard" but maybe there is a devil in some detail that is a hard requirement to you in that picture that I'm missing. Could you elaborate?

@henry2cox
Copy link
Collaborator

henry2cox commented Dec 18, 2024

Could you elaborate?

Once the vanilla testsuite is running successfully (...after I figure out why the XML generated by python3-coverage seems different than what I see locally..), then I would really like to see a 'make coverage' target that essentially does:
make COVERAGE=1 test
then publishes the ./lcov_coverage directory - possibly pointing to ./lcov_coverage/index.html as an entry point (then we can figure out what seems like a decent coverage criteria)

In at least one other framework that I have messed around in, the devil in that particular detail was to be sufficiently convincing that the whole HTML hierarchy was published (e.g., no broken links to subdirectory files).

Presuming that that works...then the next stage is to pick up data from a previous build or a release tag - and use that in a more detailed report.

(There is no particular need to wait until the tests are all running correctly - but the report might be not so helpful unless they are. More to the point: probably don't want to bother to generate coverage data unless the vanilla tests were successful.)

@hartwork
Copy link
Contributor Author

hartwork commented Dec 18, 2024

How hard is it to artifact/publish (whatever term you like) the 'test.log' file file generated by the make check target? Path appears to be /home/runner/work/lcov/lcov/tests/test.log (though not sure how the parent directory is chosen).

@henry2cox not hard, let me offer a pull request for that. Starting after this reply…

@henry2cox pull request #357 now. You can check https://github.com/hartwork/lcov-fork/actions/runs/12402870143 with a zip download at the button for what to inspect, if you like.

shot

@henry2cox
Copy link
Collaborator

FWIW: I figured out how to archive the 'tests' directory - so we can grab all the data to figure out issues.

Next step is the coverage report.
I guess, can do the same thing - and download as zip. But might be nicer to be browsable.

@hartwork
Copy link
Contributor Author

@henry2cox I'm not aware of any easy way for browsable results. Probably a zip download will have to do.

@henry2cox
Copy link
Collaborator

I found a reference to upload-pages-artifact - which I am experimenting with - no idea if I am doing the right thing or not.
As you say: easy enough to just download a zip and then browse files from there...but I kind of like the approach our jenkins uses - where I can just see the coverage reports along with test status, performance statistics, etc.
Might not be possible - but I have to believe that it is a pretty common desire, so somebody must have already done it.
Or maybe that is just for paid projects.

@hartwork
Copy link
Contributor Author

@henry2cox I'm sure it's possible but it's not something I could offer economically at the moment.

@henry2cox
Copy link
Collaborator

No worries...thanks for all the help to get as far as we got, already.
Really appreciate it.

@hartwork
Copy link
Contributor Author

@henry2cox glad to hear, glad I could be of help 🙏

@hartwork
Copy link
Contributor Author

hartwork commented Dec 21, 2024

@henry2cox our runner image offers GCC 12.3.0, 13.3.0, 14.2.0 out of the box — if we were to to use the matrix feature of GitHub Actions to cover all three versions but one, would anything else need patching — what would it need for the test suite to pick it up and use that single given GCC everywhere (outside of hacks like deleting all other available GCC binaries)?

@henry2cox
Copy link
Collaborator

henry2cox commented Dec 21, 2024

No patches that I know of offhand.
We test locally with a few other versions (mainly older, for some projects stuck with old tool chains).
The infrastructure should pick up the compiler from your environment.

If you are trying to collect cumulative coverage data (for lcov), then you need a few minor hacks to prevent overwriting the results with the next compiler.

If you are trying to collect cumulative data for your application: you just have to manage your 'aggregate' steps.
If the project is widely distributed, you probably want version checks (for your code, especially your library code).
Version mismatches can be very confusing to debug. (Then you realize what happened...d'oh.)

(Famous last words, of course... have to try and see what breaks.)

Henry

@hartwork
Copy link
Contributor Author

@henry2cox maybe I should have been more explicit: I wonder about an environment with say GCC commands gcc-12, gcc-13 and gcc-14 and gcc. I would want the CI to say "please use (say) command gcc-13" wherever you expect GCC, potentially the same for GCOV.

@henry2cox
Copy link
Collaborator

@henry2cox maybe I should have been more explicit: I wonder about an environment with say GCC commands gcc-12, gcc-13 and gcc-14 and gcc. I would want the CI to say "please use (say) command gcc-13" wherever you expect GCC, potentially the same for GCOV.

Perhaps I'm coffee deprived:
If you want to run the lcov test suite with differently named GCC compilers (gcc-13, etc) - then we need to do a bit of refactoring.

If you want to run your tests with various compilers and want to collect/collate coverage data using lcov

  • you need to tell lcov --capture how to invoke the corresponding gcov - see the --gcov-tool option (maybe write a wrapper script).
  • you are very likely to need to run your capture command once per toolchain version. Single invocation could be made to work (with a moderately complicated wrapper script) - but probably too complicated and unnecessary.

@hartwork
Copy link
Contributor Author

@henry2cox let me try a quicker hack then before we have that. Let's see if you like it…

@hartwork
Copy link
Contributor Author

hartwork commented Dec 21, 2024

@henry2cox I didn't expect my idea to perform so poorly (see draft #366). One side effect learning is that tests likely need auto-detect of -fcoverage-mcdc availability. But in particular GCC 14 I expected to work. I'm likely missing something obvious 🤔 Any ideas?

@hartwork
Copy link
Contributor Author

For anyone else reading here: #366 is more promising now.

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

No branches or pull requests

3 participants
@hartwork @henry2cox and others