hyperledger-archives / iroha

Iroha - A simple, decentralized ledger
http://iroha.tech
Apache License 2.0
988 stars 298 forks source link

Coverage: remove function signature lines that are never report as hit #2156

Open neewy opened 5 years ago

neewy commented 5 years ago

Description of the Change

Some time ago, I wanted to get the latest data about project coverage and was confused with the results. As stated in https://github.com/JeremyAgost/lcov-llvm-function-mishit-filter repo: "The lcov test coverage utility is designed to work with gcc and has some issued with running on binaries generated by the llvm compiler. One of these issues is that function signatures are output as lines that should be hit. However, while the binary is running those function signature lines are never registered as hit. Here's a simple python script to filter those lines from the lcov output."

What I did is that I added a script that would remove partial hits from function signatures. Python script won't get updated a lot, I believe; however, we should monitor lcov issue for some time here: https://github.com/linux-test-project/lcov/issues/30

Benefits

Honest coverage results

Possible Drawbacks

Possibly manual maintenance of the script is required.

Usage Examples or Tests [optional]

remove-function-lines.py lcov_output.info cleaned_output.info

neewy commented 5 years ago

@MBoldyrev TBH I did not write the code myself — I reused the script from https://github.com/JeremyAgost/lcov-llvm-function-mishit-filter repo. We can test it with your suggestions, but it seem to work now with python3.5

MBoldyrev commented 5 years ago

@MBoldyrev TBH I did not write the code myself — I reused the script from https://github.com/JeremyAgost/lcov-llvm-function-mishit-filter repo. We can test it with your suggestions, but it seem to work now with python3.5

I see, but it as fa as I can see, the code is bogous, and one of the purposes of the PR review concept is preventing bad things from entering our repo. I see that the script contains some bad stuff, and I'd like to remove it prior to merging this. The script seems simple enough to rewrite it ourselves in a more robust way.