Packit-service: Test results get blocked in "Submitting the tests ..." state

[csomh]

Happened on https://github.com/psss/tmt/pull/314, 3 out of 4 of the testing flags were not set with any result, they were showing "Pending — Build succeeded. Submitting the tests ...".

The Dashboard showed the one test that passed for this PR, but no other test was visible.

Logs did not have a trace of handling this PR at the time of checking.

[csomh]

None of the missing 3 test runs had a record in the tft_test_runs table for the relevant job_trigger_id (5305).

[psss]

I've rescheduled psss/tmt/pull/314 and all jobs seem to be stuck in Submitting tests...

[csomh]

All tests finished successfully after resubmitting them today. :man_shrugging: :confused:

[TomasTomecek]

so we good here?

[csomh]

I think yes, we are good with this particular occurrence, though I have some concerns that this might happen in the future.

The issue here was that the thing Packit communicated (the status set on the flags) was out of sync whit what it did (triggering the test) and what it remembered that it did (the thing saved in the DB).

Reading through the relevant code path the order right now is:

1. Tell the user that the tests are being submitted.
2. "Remember" that the tests are submitted.
3. Actually submit the tests.

Each of the above is a separate action and each of these actions reaches to some other system/application: GitHub API, PostgreSQL DB, Testing Farm API.

At the end of these steps we should end up with a consistent state: we told the user that we submitted the tests, we actually submitted the tests and we remembered that we submitted the tests.

But b/c each of these steps depends on some external system, the sequence is fragile. (But that's fine.) If any of the steps fail, we and up in an inconsistent state: we told the user that we submit the tests but we didn't do it and/or we don't remember that we did it (so there is no way for us to notice and recover from the inconsistent state).

I think this could be improved by changing the order of the flow above to:

1. Submit the test.
2. Take a note ("remember") that the test was submitted.
3. Notify the user that the test was submitted.

The above would ensure that users get notified about actions which really took place and are accounted for by the service.

WDYT?

If it makes sense, I would create a new issue to check all over the service that actions are done in the order above, and close this one.

[psss]

I'd say the wording Submitting the tests... describes that something is in progress (has started) rather than it's done (has finished). So I guess the wording should be updated as well.

[lachmanfrantisek]

@psss you are right, the ing form and ellipsis mean that the submission will be done.

• We are not saying that anything was done.
• We should be consistent here since we have multiple occurrences of this: Building ... / Submitting ... (as Hunor pointed out)
• Or what about two states? Submitting ... and Submitted to make it clear?

[stale[bot]]

This issue has been marked as stale because it hasn't seen any
activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed
by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are
never marked as stale.)

[csomh]

@lachmanfrantisek Should we keep this open? What do you think? If it's not a must to have an improvement in this area, I'd say to close it, as the initial issue was solved.

[lachmanfrantisek]

@csomh looks like we don't have much capacity for it. I am not against closing. Any other opinion? Or any volunteer to go through the commit state messages?

[csomh]

Let's close it then. Thanks!