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

CI coverage reports again experience random fluctuations #1256

Closed
5 of 6 tasks
fingolfin opened this issue Apr 17, 2017 · 4 comments
Closed
5 of 6 tasks

CI coverage reports again experience random fluctuations #1256

fingolfin opened this issue Apr 17, 2017 · 4 comments
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ...

Comments

@fingolfin
Copy link
Member

fingolfin commented Apr 17, 2017

Consider PR #1254, which touches no source files, yet still coverage changes. Namely in these files: (UPDATE: added more files, marked resolved files)

  • hpcgap/src/funcs.c
  • src/hpc/threadapi.c
  • hpcgap/src/vars.c
  • src/compiler.c
  • hpcgap/lib/stdtasks.g
  • stc/stats.c

It would be good if we could avoid these fluctuations, as they lead to spurious coverage failures.

@ChrisJefferson
Copy link
Contributor

ChrisJefferson commented Apr 19, 2017

The src/compiler.c issues are to do with us testing compiling mode. There are several layers of problems here:

  1. The storeString method doesn't work properly when we use it to handle -U (compiler options) -- it puts a pointer into a char [256], which we then try to parse as a string (that's where the fluctuations come from).

  2. Even if we fix it, it turns out we ignore the argument -U anyway, as we manually set the flags in InitKernel anyway.

  3. Relatedly, the -i option (set GAP root file) has been broken since at least 4.7.2 (last version I have that compiles), for the same reason.

I suggest, seeing as -U and -i haven't done anything useful for years and no-body cared, just removing them (for the compiler, in practice we know the best options to set, and can just continue hard-wiring them, as always)

@ChrisJefferson
Copy link
Contributor

This first issue (-U) is fixed in #1265. Once that gets merged, we can look to see if there are other issues as well.

@fingolfin
Copy link
Member Author

@ChrisJefferson Great find! I traced this in the logs of the old repository, and the current command line parsing code was introduced 2002-10-23. So, yeah, it should be safe to remove (although at least -i would also be trivial to fix -- but I see no point in that, and freeing up one-letter command line args to me is a good thing).

@fingolfin
Copy link
Member Author

And I just verified that at least -i was already broken in GAP 4.4.12:

~/Documents/gap4r4$ bin/gap.sh  -i lib/init.g
gap: hmm, I cannot find '�_��' maybe use option '-l <gaproot>'?
 If you ran the GAP binary directly, try running the 'gap.sh' or 'gap.bat' script instead.gap>
gap>

@wucas wucas added topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ... kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements labels Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ...
Projects
None yet
Development

No branches or pull requests

3 participants