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

Coveralls shows no coverage data #953

Closed
nlohmann opened this issue Feb 2, 2018 · 6 comments
Closed

Coveralls shows no coverage data #953

nlohmann opened this issue Feb 2, 2018 · 6 comments
Assignees
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@nlohmann
Copy link
Owner

nlohmann commented Feb 2, 2018

Since the split into the new subdirectories, Coveralls displays no coverage data, see https://coveralls.io/github/nlohmann/json. I tried to fix the .travis.yml file, but probably overlooked something.

@theodelrieu
Copy link
Contributor

Weird, I remember battling against coveralls in #700 and making it work.

Could you try to set --include include?

I think it's because in my PR, src was containing split headers, and there was a single_include folder, that has been renamed to src after the merge.

@nlohmann
Copy link
Owner Author

nlohmann commented Feb 4, 2018

But the tests are compiled using the header from the single_header directory.

@nlohmann
Copy link
Owner Author

nlohmann commented Feb 4, 2018

And I think it is more handy to have the coverage for the single header file, because this is the main artifact of the repository.

@theodelrieu
Copy link
Contributor

But having it on the include folder is way better to know where the not covered code is. When I had to open json.hpp in coveralls it froze my browser so I'd go for multiple headers all day everyday.

Is there a Travis job that runs make check-amalgamation? If that's the case, I think there is no need to run coveralls on the single header

@nlohmann
Copy link
Owner Author

nlohmann commented Feb 6, 2018

Yes, there is a job calling make check-amalgamation, so we can actually let Coveralls work on the multiple-headers version.

nlohmann added a commit that referenced this issue Feb 9, 2018
@nlohmann
Copy link
Owner Author

nlohmann commented Feb 9, 2018

Coverage data are now shown for the single-header version. I am fine with this and would close this issue. For coverage of the multi-header version, there is the make coverage target which uses lcov locally.

To extend Travis/Coveralls to use the multi-header version, we need way to call cmake with -DJSON_Coverage=ON and -DJSON_MultipleHeaders=ON - so far I only managed to pass one flag via environment variable CMAKE_OPTIONS to the call to cmake in the script passage.

@nlohmann nlohmann self-assigned this Feb 10, 2018
@nlohmann nlohmann added this to the Release 3.1.1 milestone Feb 10, 2018
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Feb 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants