flexflow / FlexFlow

FlexFlow Serve: Low-Latency, High-Performance LLM Serving
https://flexflow.readthedocs.io
Apache License 2.0
1.67k stars 224 forks source link

Add coverage metrics for unit tests in CI #1340

Closed lockshaw closed 4 months ago

lockshaw commented 6 months ago

Add coverage metrics for the unit tests currently in CI in repo-refactor, most likely along the lines described here. If generating and viewing the code coverage output will require any additional commands from FF developers, document them in the PR.

Relevant files:

Next steps:

Bob-Chen222 commented 6 months ago

Hi @lockshaw Colin, I am not sure what PR I need to make to https://github.com/lockshaw/proj because, based on my understanding of tutorial, I only need to add a few lines of code in the top-level CMakeLists.txt to enable the code coverage. (i.e. as long as adding flags when compile the whole project, I can somehow generate the code coverage report later)

After doing so, I will add some extra lines of code in the CI workflow to generate code coverage report. For example, add a script to run lcov -c -d . -o main_coverage.info \ > && genhtml main_coverage.info --output-directory out

And after that, figuring out ways to integrate output with reviewable.

Could you point out if there are any misunderstandings regarding the procedures? Thanks!

lockshaw commented 6 months ago

@Bob-Chen222 The main thing is to make sure that it's easy for developers to get the coverage information: if it's a matter of remembering to run lcov -c -d . -o main_coverage.info > && genhtml main_coverage.info --output-directory out in the right directory, tweak it a bit to find which libraries coverage you actually want to see, open a browser in the right output directory, etc. then in practice no one will use it. If it's a matter of running proj coverage and having a browser immediately open with the coverage metrics of the library being focused on, then it will be used.

tldr: the exact integration with proj, etc. doesn't really matter, the key is to make it incredibly easy for people not familiar with the system to get the coverage numbers and see them.

Bob-Chen222 commented 6 months ago

I see @lockshaw. Could you tell me how to build the project? I want to build the project and actually generate coverage report to make sure that the implementation is correct but I am not very familiar with cmake and I got a lot of errors when I use something like cd _build && cmake ..

lockshaw commented 6 months ago

@Bob-Chen222 Without more information about what system/environment you're running on I can't provide more specific direction. One option that's pretty well-tested is to use nix (or nix-portable where you don't want to install system-wide) as done in https://github.com/flexflow/sapling-guide. It's also how the CI environment on repo-refactor is configured. Happy to meet over zoom etc. if you want some help getting things set up.

lockshaw commented 4 months ago

Closed by #1396 and #1380