Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Failures later than the ~100th job are ignored #99

Closed
relsqui opened this issue May 18, 2022 · 7 comments · Fixed by #89
Closed

Failures later than the ~100th job are ignored #99

relsqui opened this issue May 18, 2022 · 7 comments · Fixed by #89
Assignees
Labels
Type: Breaking Change Creates incompatibility with current version
Projects

Comments

@relsqui
Copy link

relsqui commented May 18, 2022

Describe the bug: バグの概要

We use this action in a workflow that includes a matrix of ~180 jobs. When one of the later jobs to run fails, this action reports the workflow conclusion as a success.

To Reproduce: 再現手順

Steps to reproduce the behavior:

  1. Create a workflow with a matrix of many jobs.
  2. Include a call to workflow-conclusion-action at the end.
  3. Induce a failure in a job that will finish very late.
  4. Check workflow conclusion.

Expected behavior: 期待する動作

Workflow conclusion is a failure.

Screenshots: スクリーンショット

This is the output of workflow-conclusion-action in a workflow where a late matrix job failed.

image

Operating environment: バグが発生した環境

  • Version of software: v2
  • OS: [e.g. Windows10] ubuntu-20.04
  • Browser: [e.g. Chrome, Safari] n/a
  • etc.

Additional context: 補足

The specific numbers involved (180ish jobs, 85 truncated in the screenshot, default page size of 100) make me suspect this is pagination-related.

@technote-space
Copy link
Owner

Not reproducible

Backlog automation moved this from To do to Done Jun 1, 2022
@technote-space
Copy link
Owner

120 jobs + 2 other jobs

スクリーンショット 2022-06-02 2 14 32

スクリーンショット 2022-06-02 2 15 13

98 + 24 = 122

スクリーンショット 2022-06-02 2 14 00

@technote-space
Copy link
Owner

export const getJobs = async(octokit: Octokit, context: Context): Promise<Array<ActionsListJobsForWorkflowRunResponseData>> => octokit.paginate(

pagination

@technote-space
Copy link
Owner

Perhaps the "needs" setting is incorrect.

@technote-space
Copy link
Owner

technote-space commented Jun 1, 2022

Or same job name is used.

I don't remember why I implemented it that way at the time, but JOBs with the same name will override the status.

describe('getJobConclusions', () => {
it('should get conclusions', () => {
expect(getJobConclusions([
{name: 'test1', conclusion: 'cancelled'},
{name: 'test2', conclusion: 'neutral'},
{name: 'test3', conclusion: 'failure'},
{name: 'test4', conclusion: 'success'},
{name: 'test5', conclusion: 'failure'},
{name: 'test6', conclusion: 'success'},
{name: 'test7', conclusion: 'cancelled'},
{name: 'test8', conclusion: 'skipped'},
{name: 'test9', conclusion: 'test1'},
{name: 'test9', conclusion: 'test2'},
{name: 'test9', conclusion: 'test3'},
])).toEqual([
'cancelled',
'neutral',
'failure',
'success',
'skipped',
'test3',
]);
});
});

This behavior is confusing so I will change so that it is not overwritten.

@technote-space technote-space reopened this Jun 1, 2022
Backlog automation moved this from Done to In progress Jun 1, 2022
@github-actions github-actions bot added the Status: In Progress Tracking issues with work in progress label Jun 1, 2022
@technote-space technote-space added the Type: Breaking Change Creates incompatibility with current version label Jun 1, 2022
technote-space added a commit that referenced this issue Jun 1, 2022
BREAKING CHANGE: changed same job name behavior
@github-actions github-actions bot mentioned this issue Jun 1, 2022
@github-actions github-actions bot mentioned this issue Jun 2, 2022
1 task
Backlog automation moved this from In progress to Done Jun 3, 2022
@github-actions github-actions bot removed the Status: In Progress Tracking issues with work in progress label Jun 3, 2022
@relsqui
Copy link
Author

relsqui commented Jun 3, 2022

Thanks! I'm away from work this week but I'll check if this fixes it for us when I can.

@relsqui
Copy link
Author

relsqui commented Nov 15, 2023

TL;DR I think this is the same bug as #107 and releasing the changes from #108 will fix it.


Hiya, sorry to revive an old thread, but we just ran into this problem again so I spent a little time trying to reproduce it more reliably. I made this minimal workflow to create large matrix jobs with exactly one failure and then run workflow-conclusion-action to see what it says. I wasn't able to reproduce the bug consistently, but here are some illustrative results:

I never saw it fail on a run with fewer than 200 jobs, and never in this test repo when "fast fail" was turned off -- but in our actual work repo, we did see this error with fast fail off (and a very large matrix job). In the logs for that run, I see that the failed jobs are still marked as "in_progress," which led me to #107 .

I see that #107 was marked as completed after #108 was closed and a new version of the action was released ... but I don't see the changes from that PR anywhere in the code, in any version tag or on main. Did you intend to release that change? I think it would solve our problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Breaking Change Creates incompatibility with current version
Projects
Backlog
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants