Skip to content

Code Coverage Support#1396

Merged
lockshaw merged 30 commits intoflexflow:repo-refactorfrom
Bob-Chen222:code-coverage
May 30, 2024
Merged

Code Coverage Support#1396
lockshaw merged 30 commits intoflexflow:repo-refactorfrom
Bob-Chen222:code-coverage

Conversation

@Bob-Chen222
Copy link
Copy Markdown
Contributor

@Bob-Chen222 Bob-Chen222 commented May 29, 2024

Description of changes:

  • proj cmake will create two separate build folders. build and build_codecov. Additional flag FF_USE_CODE_COVERAGE is added to achieve the optional instrumentation in cmake
  • use proj test --coverage will generate coverage info in terminal. proj test --coverage --browser will run the report in user's browser(if have one)
  • filtered the coverage information and currently, only code in lib/ will be covered (can change the scope of coverage very easily if needed)
  • codecov is supported. The codecov report can be visualized and PR in the future will support automatic code coverage comment
  • also updated lockshaw/proj and need both to be approved to work

Related Issues:

Linked Issues:

Issues closed by this PR:


This change is Reviewable

@Bob-Chen222 Bob-Chen222 requested a review from lockshaw May 29, 2024 04:11
@Bob-Chen222 Bob-Chen222 changed the base branch from inference to repo-refactor May 29, 2024 04:11
Copy link
Copy Markdown
Collaborator

@lockshaw lockshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Bob-Chen222)


.github/workflows/helpers/cmake_cuda.sh line 28 at r1 (raw file):

        -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
        -DCMAKE_CUDA_COMPILER_LAUNCHER=ccache \
        "${FLAGS[@]}"

Suggestion:

../config/config.linux \
        -DCMAKE_C_COMPILER_LAUNCHER=ccache \
        -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
        -DCMAKE_CUDA_COMPILER_LAUNCHER=ccache \
        -DFF_USE_CODE_COVERAGE=ON \
        "${FLAGS[@]}"

Copy link
Copy Markdown
Contributor Author

@Bob-Chen222 Bob-Chen222 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 6 of 7 files reviewed, 1 unresolved discussion (waiting on @lockshaw)


.github/workflows/helpers/cmake_cuda.sh line 28 at r1 (raw file):

        -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
        -DCMAKE_CUDA_COMPILER_LAUNCHER=ccache \
        "${FLAGS[@]}"

Done.

@Bob-Chen222 Bob-Chen222 requested a review from lockshaw May 30, 2024 02:56
lockshaw
lockshaw previously approved these changes May 30, 2024
Copy link
Copy Markdown
Collaborator

@lockshaw lockshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Bob-Chen222)

lockshaw
lockshaw previously approved these changes May 30, 2024
Copy link
Copy Markdown
Collaborator

@lockshaw lockshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Bob-Chen222)

Copy link
Copy Markdown
Collaborator

@lockshaw lockshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Bob-Chen222)


.github/workflows/per-lib-check.yml line 115 at r4 (raw file):

          verbose: true
          
      - name: Test substitution-generator

You probably want to run all tests before uploading the codecov info?

Copy link
Copy Markdown
Collaborator

@lockshaw lockshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Bob-Chen222)

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.

Optional CodeCoverage Building Instrumentation

3 participants