[PATCH] D154280: [LIT] Added an option to llvm-lit to emit the necessary test coverage data, divided per test case

[Shivam Gupta via Phabricator via llvm-commits]

xgupta added a comment.

> @xgupta Could you add a test case where a lit.cfg file sets config.per_test_coverage directly? A test suite may want to set it there rather than on the command line.

I have added one test case but it is not passing, I am investigating that.



================
Comment at: llvm/docs/CMake.rst:378
 
+**LLVM_TEST_COVERAGE**:BOOL
+  Enable individual test case coverage. When set to ON, code coverage data
----------------
delcypher wrote:
> This documentation seems inaccurate right now. Reading the implementation, enabling this option doesn't actually cause any coverage to be collected. I suspect you will do this in later patches but I would suggest you update the documentation when the feature actually works.
> 
> Nits:
> 
> * `LLVM_TEST_COVERAGE` is a vague name. `LLVM_INDIVIDUAL_TEST_COVERAGE` might be more specific.
> * Consider being more specific about where the code coverage data is written (e.g. under the `config.test_exec_root` directory.
> * Documentation doesn't typically use the phrase "This allows you to...". Instead consider writing " This allows code coverage analysis of each individual test case"
> 
> Super nit:
> 
> * Missing fullstop (period) at the end of the last sentence.
> 
> This documentation seems inaccurate right now. Reading the implementation, enabling this option doesn't actually cause any coverage to be collected. I suspect you will do this in later patches but I would suggest you update the documentation when the feature actually works.

Yeah, I was thinking of future name, agree, updated now, Thanks.


================
Comment at: llvm/docs/CommandGuide/lit.rst:193
+ setting a unique value to LLVM_PROFILE_FILE for each RUN). The coverage
+ data files will be emitted in the <build>/test/ directory.
+
----------------
delcypher wrote:
> You may want to rephrase this. Lit can (and is) used outside of the llvm project so the `<build>/test` directory isn't guaranteed to be the name of a test directory. Instead I think you're saying that the coverage data files will be located in the directory that `config.test_exec_root` is set to in a test suite.
> 
> A concrete example under the `llvm-project` itself would be clang. It's lit shell tests are usually located in `<build>/tools/clang/test` which is different to the path you give above.
Thanks for pointing it, updated.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:1132
+            # Set the LLVM_PROFILE_FILE environment variable
+            llvm_profile_file = test_case_name + ".profraw"
+            env = test.config.environment
----------------
delcypher wrote:
> How is this unique to every `RUN` line? A test case could have multiple run lines and I don't see how that would be handled here.
I updated the documentation to say it is set for each test file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154280/new/

https://reviews.llvm.org/D154280