1465161 - [checks] Retriggers don't change check result

Status: RESOLVED INCOMPLETE

(Taskcluster :: Services, enhancement, P3)

Description

[Brian Stack [:bstack]]

Copied from https://github.com/taskcluster/taskcluster-github/issues/235

I can provide further guidance as needed, but the general idea would be to update [0] such that if a task has the same name as a previous task in that group and has succeeded, overwrite the failure with success!

[0] https://github.com/taskcluster/taskcluster-github/blob/ac35fe30ead836c34db9905ea867a03d3b2d1d42/src/handlers.js#L191-L194

Comment 1

[Arshad Kazmi [:arshadkazmi42]]

I can take this up.

For this I need to merge the state of tasks with same name and if any of the duplicate task has status success, I should update all the task with the same name as success.

AM I on the right path? 
Also is there any test which needs to be verified for this

Comment 2

[Brian Stack [:bstack]]

Ah great! I think we can even simplify this and say that we overwrite whatever the previous result was whether its pass or fail.

There are no tests for this right now, I think you'll need to create some new ones! Let me know how else I can help.

Comment 3

[Arshad Kazmi [:arshadkazmi42]]

(In reply to Brian Stack [:bstack] from comment #2)
> Ah great! I think we can even simplify this and say that we overwrite
> whatever the previous result was whether its pass or fail.
> 
> There are no tests for this right now, I think you'll need to create some
> new ones! Let me know how else I can help.

So I guess I need to just remove this condition

`
_.includes(['failed', 'exception'], task.status.state)
`

and add `task.state.state` to state variable directly

Is there any sample test which I can refer for writing test for this. As this is my first time working on this repo

Comment 4

[Arshad Kazmi [:arshadkazmi42]]

Also, how can I setup this in my local machine, to test it?

Is there some docs to do the setup?

Comment 5

[Brian Stack [:bstack]]

You will also need to add code to account for task names rather than just taskId. You should be able to run the tests just by `yarn` and then `yarn test`. There are mocks of the github api in [1] that you can use.
 

[1] https://github.com/taskcluster/taskcluster-github/blob/master/test/github-auth.js

Comment 6

[Arshad Kazmi [:arshadkazmi42]]

(In reply to Brian Stack [:bstack] from comment #5)
> You will also need to add code to account for task names rather than just
> taskId. You should be able to run the tests just by `yarn` and then `yarn
> test`.

I didn't get this, where I need to store task names?
In the [0] I do not see any name related variables.


> [1]
> https://github.com/taskcluster/taskcluster-github/blob/master/test/github-
> auth.js

I need to write similar test case or use just need to verify the existing test is passed or not?


[0] https://github.com/taskcluster/taskcluster-github/blob/ac35fe30ead836c34db9905ea867a03d3b2d1d42/src/handlers.js#L191-L194

Comment 7

[Brian Stack [:bstack]]

You can find the name in task.metadata.name in [2].

You'll have to create a new test that runs a task a second time with a different result and assert that the old result was updated.



[2] https://docs.taskcluster.net/docs/reference/platform/taskcluster-queue/references/api#list-task-group

Comment 8

[Arshad Kazmi [:arshadkazmi42]]

Where I have to use that name? 

is it in [3]

[3] https://github.com/taskcluster/taskcluster-github/blob/ac35fe30ead836c34db9905ea867a03d3b2d1d42/src/handlers.js#L226

Comment 9

[Arshad Kazmi [:arshadkazmi42]]

i am really new to this project, never used it before. So feel like a bit lost.

Comment 10

[Brian Stack [:bstack]]

Unfortunately I'm not sure exactly where you'll need to use it either. Probably a good next step is to try some things out and see what results you get. Probably write a test first that does something simple and see if you can get it to pass. The documentation of the library we use to talk to github might be helpful as well!

Comment 11

[Arshad Kazmi [:arshadkazmi42]]

I made changes in the line number mentioned in Comment 0
something like this 

const taskState = _.get(task, 'status.state');
if (taskState) {
 state = taskState;
}

and i ran existing tests using yarn test. those tests are passing.

How can I run a task in the test? is it something similar to this 

[4] https://github.com/taskcluster/taskcluster-github/blob/master/test/github-auth.js
[5] https://github.com/taskcluster/taskcluster-github/blob/master/test/handler_test.js

Comment 13

[Chris Cooper [:coop] (he/him)]

owlish: is there a justification for WONTFIXing this bug? I just spoke to catlee today, and this issue is annoying for releng to deal with.

If it's very hard/impossible, WONTFIX is fine, but otherwise we should re-open and just set the expectation that we won't get to it for a while.

Comment 14

[[:owlish] 🦉 PST]

Brian, Dustin and I discussed this bug, and it seems to us there's little sense in trying to fix it in Statuses API, but in order to roll out Checks they need to be more ready (which we won't get to for a while). Now that I look at the bug, I am realizing we thought this was specifically for Statuses API, but actually this is a general request, and I am also noticing it depends on Checks tracking bug.

I am going to re-open it, and link it to two other related bugs.

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

Newer work on tracking checks API is in GitHub now.