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

Options --scan-exclusion-markers/--skip-exclusion-markers and --validate-sources #81

Open
johan-boule opened this issue Jan 25, 2022 · 5 comments

Comments

@johan-boule
Copy link

In a CI scenario, I've encountered a problem after a .cpp source file was removed from the repository.
We use incremental builds on the CI server and our build system is far from perfect (CMake), so it's not removing the derived files that where produced by the former .cpp, i.e., we still have the .o and .gcno files lying around.
When fastcov is called without the --skip-exclusion-markers, it's bailing out because it can't find the source file.

Perhaps fastcov's behaviour could be adjusted so that it doesn't fail in this case unless the --validate-sources option was also used ?

johan-boule pushed a commit to johan-boule/fastcov that referenced this issue Jan 25, 2022
…sion markers and source file isn't found unless the --validate-sources option was passed
@RPGillespie6
Copy link
Owner

RPGillespie6 commented Jan 25, 2022

I guess the question is... will this proposed change cause undesirable behavior in anyone else's CI pipelines? Hard to know if other people might be assuming and relying on fastcov failing if there is a missing exclusion source file by default.

What if instead we just add an option --skip-missing-exclusion-sources which, if set, won't write a non-zero exit code if an exclusion source couldn't be found?

That will also prevent fastcov tests from failing. We can add a new test for the flag here: https://github.com/RPGillespie6/fastcov/blob/master/test/functional/run_all.sh#L163

We can basically just copy paste that test, but add --skip-missing-exclusion-sources and then check test $rc -eq 0

If you update your PR to do that I'll merge it.

@snakethatlovesstaticlibs
Copy link

snakethatlovesstaticlibs commented Jan 6, 2023

Hi @RPGillespie6, I'm also looking for this feature. If I implement it, are you open to merging it?

Edit: Actually --skip-exclusion-markers might fix my use case

@RPGillespie6
Copy link
Owner

Sure, open to PRs. Feature would need to be backwards compatible with existing users, which is why I recommended making a new flag --skip-missing-exclusion-sources

@johan-boule
Copy link
Author

johan-boule commented Jan 6, 2023

Sorry to have left this as is without further work on implementing the suggestion for a new command line flag.
I also ended up just putting the skip-exclusion-marker option.

@snakethatlovesstaticlibs

No worries! I also used `--ignore-errors source" for genhtml (for future readers). Thanks for the great library @RPGillespie6! It's greatly sped up our pipeline

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