Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #5486 & part of #5343: Introducing new wiki page for code coverage usage and limitations #5483

Merged
merged 67 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
5ad95d0
Introducing new wiki page for code coverage
Rd4dev Aug 10, 2024
dae2add
How to use code coverage tool - wiki additions
Rd4dev Aug 12, 2024
a2966e8
Limitations section code coverage
Rd4dev Aug 12, 2024
c6ebcff
Adding page for Writing test with good behavioural coverage
Rd4dev Aug 13, 2024
51393a4
Rewrote Writing tests with good behavioural coverage wiki page
Rd4dev Aug 14, 2024
161c7fd
Merge with develop after merge of upload comments pr
Rd4dev Aug 14, 2024
9630d7f
Updates as per reviews part 1
Rd4dev Aug 14, 2024
3f34443
Trying to fix the failure of accessing evaluate job if files are skipped
Rd4dev Aug 14, 2024
fd3675d
Check coverage status pass with skip files present
Rd4dev Aug 14, 2024
e97b1ab
Updates as per reviews part 2 with addition to coverage summary, unit…
Rd4dev Aug 14, 2024
15e1033
Testing from forked repo to see if permissions work
Rd4dev Aug 14, 2024
1cad5c6
Replacing pull request with pull request target
Rd4dev Aug 14, 2024
f9b4b0d
On pull request target with opened, synchronize and reopened
Rd4dev Aug 14, 2024
6ec1790
Trying to fix syntax error with workflow
Rd4dev Aug 14, 2024
a6ce375
Remove comments
Rd4dev Aug 14, 2024
195e34d
With just pr target set
Rd4dev Aug 14, 2024
3bc2729
Only on push
Rd4dev Aug 14, 2024
3f9ee1e
Reverting to the pull_request
Rd4dev Aug 14, 2024
5c993dc
With just pr target
Rd4dev Aug 14, 2024
a11422f
Pull request target
Rd4dev Aug 15, 2024
65345ee
Adding push trigger
Rd4dev Aug 15, 2024
f9455dc
Moving the nested pull
Rd4dev Aug 15, 2024
dad7858
Pull Request target labels
Rd4dev Aug 15, 2024
d6ed539
Removing push trigger
Rd4dev Aug 15, 2024
bf845b7
Adding Permissions to write
Rd4dev Aug 15, 2024
b8ca56b
To try with manual triggers
Rd4dev Aug 15, 2024
8c67fff
Do versions make any changes
Rd4dev Aug 15, 2024
813ff66
Manually providing the issue number
Rd4dev Aug 15, 2024
a5aeb12
Pull Request target
Rd4dev Aug 15, 2024
b58db86
Addressing Review part 1 - Exception messages and fixes
Rd4dev Aug 16, 2024
1c1531d
Addressing reviews part 2 - Structuring Test Functionalities
Rd4dev Aug 17, 2024
f7552b3
Includes minor changes and removal of pull_request_target triggers to…
Rd4dev Aug 17, 2024
b1a5bd7
Addressed 2nd Pass of review - API Testing, Quality
Rd4dev Aug 17, 2024
fba4b86
Split code coverage workflow to 2 operations to handle analysis, eval…
Rd4dev Aug 17, 2024
1770f0a
Reverting the new line change in CoverageReporter - introduced for te…
Rd4dev Aug 17, 2024
41cda2a
Added a reference footnote to the Oppia Android Code Coverage Wiki page
Rd4dev Aug 18, 2024
de40692
Evaluation job moved to the coverage report workflow to aid in skip j…
Rd4dev Aug 18, 2024
6989c6c
Code Coverage Status Check on pb file presence, Lint fixes
Rd4dev Aug 18, 2024
a2e2c7a
Removed empty pb file output variable
Rd4dev Aug 19, 2024
1ae73a2
Removed concurrency as it fails every concurrent run of the workflow …
Rd4dev Aug 19, 2024
e04cd2b
Indentation fix
Rd4dev Aug 19, 2024
ef924f1
Fixed concurrency issues with uploading comments
Rd4dev Aug 19, 2024
a4c48d6
Added missed hasMessageThat() assertion chain
Rd4dev Aug 20, 2024
f8da0f5
Test changes
Rd4dev Aug 20, 2024
251cfd2
Dropping unnecessary changes committed with wrong branch refernce
Rd4dev Aug 20, 2024
c6ed248
Structuring test bodies, isEqualTo()
Rd4dev Aug 20, 2024
8dd0eb2
Removed sh highlighting on illustrations and replaced strings with co…
Rd4dev Aug 20, 2024
c102d7f
A public api, permalink, a bank account
Rd4dev Aug 20, 2024
e575034
Replaced textual illustration with mermaid blocks
Rd4dev Aug 20, 2024
89f9d9b
Replaced textual representation of procedure to test public api proce…
Rd4dev Aug 20, 2024
489a08b
Represent Structuring and mapping with mermaid graphs
Rd4dev Aug 20, 2024
509e3c3
Common color codes for all graph illustrations
Rd4dev Aug 20, 2024
d67e4d0
Added code coverage analysis to the review process
Rd4dev Aug 20, 2024
8f25484
Standard usage of behavior
Rd4dev Aug 20, 2024
dab4faa
Updted test name with multiple conditions, specific use of words
Rd4dev Aug 20, 2024
38c0a5c
Moved @Test annotation and function to its own line
Rd4dev Aug 21, 2024
bcfc30f
Updating the helper function example to use log file utility case
Rd4dev Aug 21, 2024
3e8a57c
Added source file changes to helper function changes
Rd4dev Aug 21, 2024
e4eb862
Setting the text color for the graphs to black as they weren't visibl…
Rd4dev Aug 21, 2024
c9a6df7
Updated the side bar info too to align with contents
Rd4dev Aug 21, 2024
13d566e
Added missed comma on parameter declaration
Rd4dev Aug 21, 2024
a56c40a
Added missed permalink for FractionParser
Rd4dev Aug 21, 2024
6c29802
Copy pasted the exact code for code coverage and coverage report from…
Rd4dev Aug 21, 2024
9bfd4c2
Fix remaining nits with punctuations, plural forms and empty check
Rd4dev Aug 21, 2024
98587f3
Moved the evaluation and check status back to their original workflow…
Rd4dev Aug 23, 2024
1ac4b7c
Merge with develop branch
Rd4dev Aug 23, 2024
e6710b3
Merge branch 'develop' into code_coverage_wiki_instruction
BenHenning Aug 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ on:
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

jobs:
Expand Down
33 changes: 7 additions & 26 deletions .github/workflows/code_coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ on:
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

jobs:
Expand Down Expand Up @@ -255,10 +255,13 @@ jobs:
name: coverage-report-${{ env.SHARD_NAME }} # Saving with unique names to avoid conflict
path: coverage_reports

evaluate-code-coverage-reports:
evaluate_code_coverage_reports:
name: Evaluate Code Coverage Reports
runs-on: ubuntu-20.04
needs: code_coverage_run
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
env:
CACHE_DIRECTORY: ~/.bazel_cache
steps:
Expand Down Expand Up @@ -305,32 +308,10 @@ jobs:
name: final-coverage-report
path: coverage_reports/CoverageReport.md

publish_coverage_report:
name: Publish Code Coverage Report
needs: evaluate-code-coverage-reports
permissions:
pull-requests: write

# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
runs-on: ubuntu-latest
steps:
- name: Download Generated Markdown Report
uses: actions/download-artifact@v4
with:
name: final-coverage-report

- name: Upload Coverage Report as PR Comment
uses: peter-evans/create-or-update-comment@v4
with:
issue-number: ${{ github.event.pull_request.number }}
body-path: 'CoverageReport.md'

# Reference: https://github.community/t/127354/7.
check_coverage_results:
name: Check Code Coverage Results
needs: [ compute_changed_files, code_coverage_run, evaluate-code-coverage-reports ]
needs: [ compute_changed_files, code_coverage_run, evaluate_code_coverage_reports ]
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
Expand All @@ -341,5 +322,5 @@ jobs:
run: exit 1

- name: Check that coverage status is passed
if: ${{ needs.evaluate-code-coverage-reports.result != 'success' }}
if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.evaluate_code_coverage_reports.result != 'success' }}
run: exit 1
81 changes: 81 additions & 0 deletions .github/workflows/comment_coverage_report.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# Contains jobs corresponding to publishing coverage reports generated by code_coverage.yml.

name: Comment Coverage Report

# Controls when the action will run. Triggers the workflow on pull request events
# (assigned, opened, synchronize, reopened)

on:
pull_request_target:
types: [assigned, opened, synchronize, reopened]

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

jobs:
check_code_coverage_completed:
name: Check code coverage completed
runs-on: ubuntu-latest
steps:
- name: Wait for code coverage to complete
id: wait-for-coverage
uses: ArcticLampyrid/[email protected]
with:
workflow: code_coverage.yml
sha: auto
allowed-conclusions: |
success
failure

comment_coverage_report:
name: Comment Code Coverage Report
needs: check_code_coverage_completed
permissions:
pull-requests: write

# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
runs-on: ubuntu-latest
steps:
- name: Find CI workflow run for PR
id: find-workflow-run
uses: actions/github-script@v7
continue-on-error: true
with:
script: |
// Find the last successful workflow run for the current PR's head
const { owner, repo } = context.repo;
const runsResponse = await github.rest.actions.listWorkflowRuns({
owner,
repo,
workflow_id: 'code_coverage.yml',
event: 'pull_request',
head_sha: '${{ github.event.pull_request.head.sha }}',
});

const runs = runsResponse.data.workflow_runs;
runs.sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime());

const run = runs[0];
if(!run) {
core.setFailed('Could not find a succesful workflow run for the PR');
return;
}

core.setOutput('run-id', run.id);

- name: Download Generated Markdown Report
uses: actions/download-artifact@v4
if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status
with:
name: final-coverage-report
github-token: ${{ secrets.GITHUB_TOKEN }}
run-id: ${{ steps.find-workflow-run.outputs.run-id }}

- name: Upload Coverage Report as PR Comment
uses: peter-evans/create-or-update-comment@v4
with:
issue-number: ${{ github.event.pull_request.number }}
body-path: 'CoverageReport.md'
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

# This workflow has the following jobs:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ on:
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

jobs:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: true

jobs:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,16 +496,37 @@ class CoverageReporter(
}
}

val finalReportText = "## Coverage Report\n\n" +
"### Results\n" +
"Number of files assessed: ${coverageReportContainer.coverageReportList.size}\n" +
"Overall Coverage: **${"%.2f".format(calculateOverallCoveragePercentage())}%**\n" +
"Coverage Analysis: $status\n" +
"##" +
failureMarkdownTable +
failureMarkdownEntries +
successMarkdownEntries +
testFileExemptedSection
val wikiPageLinkNote = buildString {
val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" +
"(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page"
append("\n\n")
append("#")
append("\n")
append(wikiPageReferenceNote)
}

val skipCoverageReportText = buildString {
append("## Coverage Report\n")
append("### Results\n")
append("Coverage Analysis: **SKIP** :next_track_button:\n\n")
append("_This PR did not introduce any changes to Kotlin source or test files._")
append(wikiPageLinkNote)
}

val finalReportText = coverageReportContainer.coverageReportList.takeIf { it.isNotEmpty() }
?.let {
"## Coverage Report\n\n" +
"### Results\n" +
"Number of files assessed: ${coverageReportContainer.coverageReportList.size}\n" +
"Overall Coverage: **${"%.2f".format(calculateOverallCoveragePercentage())}%**\n" +
"Coverage Analysis: $status\n" +
"##" +
failureMarkdownTable +
failureMarkdownEntries +
successMarkdownEntries +
testFileExemptedSection +
wikiPageLinkNote
} ?: skipCoverageReportText

val finalReportOutputPath = mdReportOutputPath
?.let { it }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class RunCoverageTest {
append("| File | Failure Reason | Status |\n")
append("|------|----------------|--------|\n")
append("| ${getFilenameAsDetailsSummary(sampleFile)} | $failureMessage | :x: |")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown)
Expand Down Expand Up @@ -229,6 +230,7 @@ class RunCoverageTest {
append("\n\n")
append(exemptionsReferenceNote)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -278,6 +280,7 @@ class RunCoverageTest {
append("\n\n")
append(exemptionsReferenceNote)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -385,6 +388,7 @@ class RunCoverageTest {
append("| File | Failure Reason | Status |\n")
append("|------|----------------|--------|\n")
append("| //coverage/example:AddNumsTest | $failureMessage | :x: |")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown)
Expand Down Expand Up @@ -469,6 +473,7 @@ class RunCoverageTest {
append("| File | Failure Reason | Status |\n")
append("|------|----------------|--------|\n")
append("| //coverage/test/java/com/example:SubNumsTest | $failureMessage | :x: |")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown)
Expand Down Expand Up @@ -655,6 +660,7 @@ class RunCoverageTest {
":white_check_mark: | $MIN_THRESHOLD% |\n\n"
)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -843,6 +849,7 @@ class RunCoverageTest {
"| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 0.00% | 0 / 4 | " +
":x: | $MIN_THRESHOLD% |\n"
)
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -934,6 +941,7 @@ class RunCoverageTest {
":x: | 101% _*_ |"
)
append("\n\n>**_*_** represents tests with custom overridden pass/fail coverage thresholds")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1021,6 +1029,7 @@ class RunCoverageTest {
)
append("\n\n>**_*_** represents tests with custom overridden pass/fail coverage thresholds\n")
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1090,6 +1099,7 @@ class RunCoverageTest {
":white_check_mark: | $MIN_THRESHOLD% |\n\n"
)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1162,6 +1172,7 @@ class RunCoverageTest {
append("\n\n")
append(exemptionsReferenceNote)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1238,6 +1249,7 @@ class RunCoverageTest {
append("\n\n")
append(exemptionsReferenceNote)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1333,6 +1345,7 @@ class RunCoverageTest {
append("\n\n")
append(exemptionsReferenceNote)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1438,6 +1451,7 @@ class RunCoverageTest {
append("\n\n")
append(exemptionsReferenceNote)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1642,6 +1656,7 @@ class RunCoverageTest {
":white_check_mark: | $MIN_THRESHOLD% |\n\n"
)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -1722,6 +1737,7 @@ class RunCoverageTest {
":white_check_mark: | $MIN_THRESHOLD% |\n\n"
)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

assertThat(readFinalMdReport()).isEqualTo(expectedResult)
Expand Down Expand Up @@ -2282,6 +2298,7 @@ class RunCoverageTest {
"3 / 4 | :white_check_mark: | $MIN_THRESHOLD% |\n\n"
)
append("</details>")
append(oppiaCoverageWikiPageLinkNote)
}

return markdownText
Expand Down Expand Up @@ -2575,13 +2592,20 @@ class RunCoverageTest {
}

private fun getFilenameAsDetailsSummary(
filePath: String,
additionalData: String? = null
filePath: String
): String {
val fileName = filePath.substringAfterLast("/")
val additionalDataPart = additionalData?.let { " - $it" } ?: ""

return "<details><summary><b>$fileName</b>$additionalDataPart</summary>$filePath</details>"
return "<details><summary><b>$fileName</b></summary>$filePath</details>"
}

private val oppiaCoverageWikiPageLinkNote = buildString {
val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" +
"(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page"
append("\n\n")
append("#")
append("\n")
append(wikiPageReferenceNote)
}

private fun loadCoverageReportProto(
Expand Down
Loading
Loading