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

support lcov files without TN #104

Merged
merged 1 commit into from
Dec 7, 2024

Conversation

Cheekie25
Copy link
Contributor

Some lcov files don't have a TN: field.
The parser currently crashes in this case.
Just set current_test_name to empty at the beginning to handle this case.

The documentation
https://manpages.ubuntu.com/manpages/noble/man1/geninfo.1.html
seems to say that the testname is indeed optional:

If available, a tracefile begins with the testname which is stored in the following format:

         TN:<test name>

@RPGillespie6
Copy link
Owner

RPGillespie6 commented Dec 6, 2024

Thanks for this, do you mind adding a test case for this to run_all.sh?

You can just add a test1.no_tn.info to https://github.com/RPGillespie6/fastcov/tree/master/test/functional/expected_results

And then add a test case here:
https://github.com/RPGillespie6/fastcov/blob/master/test/functional/run_all.sh#L131

# Combine operation - Missing TN
coverage run -a ${TEST_DIR}/fastcov.py -C ${TEST_DIR}/expected_results/test1.no_tn.info --lcov -o test1.no_tn.actual.info
cmp test1.no_tn.actual.json ${TEST_DIR}/expected_results/combine5.expected.info

If not I can merge and do the test case myself

Some lcov files don't have a `TN:` field.
The parser currently crashes in this case.
Just set `current_test_name` to empty at the beginning
to handle this case.

The documentation
https://manpages.ubuntu.com/manpages/noble/man1/geninfo.1.html
seems to say that the testname is indeed optional:

```
If available, a tracefile begins with the testname which is stored in the following format:

         TN:<test name>
```
@RPGillespie6 RPGillespie6 merged commit 3b0abdb into RPGillespie6:master Dec 7, 2024
1 check passed
@RPGillespie6
Copy link
Owner

Looks good, thanks for adding the test.

@Cheekie25
Copy link
Contributor Author

Looks good, thanks for adding the test.

Thank you.
Would you mind releasing a new version with those two changes ?

@RPGillespie6
Copy link
Owner

Done

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

Successfully merging this pull request may close these issues.

2 participants