Test-infra: pr status seems to put "In Pool - Test failed/Missing Labels" when tide is retesting

[BenTheElder]

I'm pretty sure the PR didn't fail and isn't missing labels but it has "In Pool - Test failed/Missing Labels" as the status

/kind bug
/area prow
/area prow/deck
cc @qhuynh96 @amwat

[BenTheElder]

it later switched to "In Pool - In Batch & Test Pending" fwiw

[stevekuznetsov]

Sounds like we could use some unit tests for that JS logic :grimace:

[BenTheElder]

@Katharine was planning to propose modernizing the JS hopefully

Edit: wrong spelling, wrong Katharine :( sorry
On Fri, Sep 14, 2018, 19:15 Steve Kuznetsov [e-mail]
wrote:

Sounds like we could use some unit tests for that JS logic :grimace:

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/test-infra/issues/9413#issuecomment-421523702,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4Bq9C7gs_jFjxO4NbcZKKpKYXE5l2Bks5ubGLSgaJpZM4WqJk-
.

[qhuynh96]

The In Pool - Test failed/Missing Labels label appears when the PR falls into pool.MissingPrs category. Is there any other reason, except test failed/ missing labels, that makes a PR falls into pool.MissingPrs?

[BenTheElder]

@cjwagner for tide ^

[cjwagner]

The In Pool - Test failed/Missing Labels label appears when the PR falls into pool.MissingPrs category. Is there any other reason, except test failed/ missing labels, that makes a PR falls into pool.MissingPrs?

I think there is confusion about how Tide categorizes PRs. The category names are somewhat vestigial which doesn't help.
pool.MissingPrs contains PRs that are in the pool, but are missing up to date test results (i.e. the prowjobs were GCed or invalidated by another PR merging). It never contains PRs that fail tests or are missing labels.

In fact Test failed and Missing Labels don't make sense for anything in the pool including PRs in MissingPrs. If a PR fails required tests it is removed from the pool. If a PR does not meet label requirements it is removed from the pool.
Test failed and Missing Labels should only ever apply to PRs that are not in the pool.

[qhuynh96]

Thanks Cole! So it was named incorrectly. I would renamed it to In pool / Queued for retest to match tide page. WDYT?

[cjwagner]

I would renamed it to In pool / Queued for retest to match tide page. WDYT?

SGTM, +1 for using consistent phrases across pages.