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

[BUG]: Code Coverage comment fails with disallowed conclusion: action_required #5566

Closed
Rd4dev opened this issue Nov 4, 2024 · 5 comments · Fixed by #5574
Closed

[BUG]: Code Coverage comment fails with disallowed conclusion: action_required #5566

Rd4dev opened this issue Nov 4, 2024 · 5 comments · Fixed by #5574
Assignees
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Low Solution is clear and broken into good-first-issue-sized chunks.

Comments

@Rd4dev
Copy link
Collaborator

Rd4dev commented Nov 4, 2024

Describe the bug

The PR #5560, #5559 fails to comment the generated code coverage report.

The stack trace mentions:

Error: Run#11587680371 is completed with disallowed conclusion: action_required

Suspect:

Since a review was requested and the trigger relies on the pull request status, it may have marked the action as "required" rather than as a success or failure, which are currently the only two permitted conclusions for the coverage report. (Earlier reviews were submitted as comments, so they didn't lead to this issue.)

Updated Findings:

I initially thought the action_required status might have been triggered by a review request, but during testing, I was able to replicate it by setting rules to manually require workflow approvals. This makes me think the issue could indeed be related to the approval requirements (notably, both PRs where this occurred required workflow approval).

Given this, simply adding an action_required conclusion or adjusting the check run handling might not fully resolve the issue. None of the reproductions were conclusive, so a more thorough investigation is needed to identify and resolve the underlying cause.

Steps To Reproduce

  • Create a PR and request a review
  • Ensure this review is of the type that requires an action ("Request Changes") instead of just adding comments.
  • Once the review is submitted, it should be marked as "action_required" in the comment coverage stack trace

Expected Behavior

It should get a success conclusion and post a coverage comment with the generated coverage data.

Screenshots/Videos

No response

What device/emulator are you using?

No response

Which Android version is your device/emulator running?

No response

Which version of the Oppia Android app are you using?

No response

Additional Context

Adding "action_required" to the desired conclusions could be a solution if that's the only cause of this issue, yet it need to be validated.

@Rd4dev Rd4dev added bug End user-perceivable behaviors which are not desirable. triage needed labels Nov 4, 2024
@Rd4dev Rd4dev self-assigned this Nov 4, 2024
@adhiamboperes adhiamboperes added Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Low Solution is clear and broken into good-first-issue-sized chunks. and removed triage needed labels Nov 5, 2024
@manas-yu
Copy link
Collaborator

manas-yu commented Nov 7, 2024

@adhiamboperes can i work on this issue? We can fix this issue by explicitly handling the action_required status in the check_coverage_results section of the code_coverage.yml file.

- name: Handle action required status
        if: ${{ needs.code_coverage_run.result == 'action_required' || needs.evaluate_code_coverage_reports.result == 'action_required' }}
  run: |
    echo "Action required on code coverage. Review manually before proceeding."
 exit 0

@Rd4dev Rd4dev assigned manas-yu and unassigned Rd4dev Nov 8, 2024
@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Nov 8, 2024

@manas-yu, thank you for looking into this! Assigning this issue to you to let you take over.

I’d suggest prioritizing a straightforward approach by adding 'action_required' to allowed_conclusion and testing if that resolves it. Also, please verify separately that code coverage still runs and posts comments, as this is needed even when a PR review request is made.

@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Nov 9, 2024

Updated Findings:

I initially thought the action_required status might have been triggered by a review request, but during testing, I was able to replicate it by setting rules to manually require workflow approvals. This makes me think the issue could indeed be related to the approval requirements (notably, both PRs where this occurred required workflow approval).

Given this, simply adding an action_required conclusion or adjusting the check run handling might not fully resolve the issue. None of the reproductions were conclusive, so a more thorough investigation is needed to identify and resolve the underlying cause.

@BenHenning
Copy link
Member

From discussing this in the CLaM meeting, I'm starting to think that it many not be possible to fully fix this at this time. Some thoughts:

I think if we can handle the failure case a bit better, this might not be a bad outcome since it should only affect new contributors who need their workflows approved.

@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Nov 12, 2024

Just for clarity, the utilized external action is: https://github.com/ArcticLampyrid/action-wait-for-workflow

Filed the issue: ArcticLampyrid/action-wait-for-workflow#269

subhajitxyz pushed a commit to subhajitxyz/oppia-android that referenced this issue Nov 19, 2024
…#5574)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes oppia#5566: This PR addresses the issue of code coverage comments not
being posted on pull requests when certain review status ("Request
Changes") result in an `action_required` status.

### Changes:
- Updated `comment_coverage.yml` to include `action_required` in the
`allowed-conclusions` list for the `check_code_coverage_completed` job.
This ensures that the workflow continues even if a review requires
further action.
## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Work: Low Solution is clear and broken into good-first-issue-sized chunks.
Development

Successfully merging a pull request may close this issue.

4 participants