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

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
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.
+
----------------
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.


================
Comment at: llvm/utils/lit/lit/LitConfig.py:91
         self.echo_all_commands = echo_all_commands
+        self.per_test_coverage = per_test_coverage
 
----------------
Consider adding a `@property` getter and a setter (see `maxIndividualTestTime`) so that when setting the data it can be validated.


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154280