Checks plugin question: how to tailor checker to specific conditions

41 views
Skip to first unread message

lucamilanesio

unread,
Apr 17, 2020, 7:22:03 AM4/17/20
to Repo and Gerrit Discussion
Hi Checks plugin developers,
we have on the Gerrit CI checkers for the backend (Build/Test) and the PolyGerrit UI (PolyGerrit UI Test), however, not every change needs to perform both.

For example [1] is a PolyGerrit UI only change and therefore we don't perform the backend tests, because there were no changes on the backend.
Is there any way to do this "negative logic" for the Checker? (E.g. Do not trigger this Checker if condition XYZ)

AFAIK the only thing that checkers can do at the moment is associating to a Gerrit change query (see [2]) which may not give the expected result.

Thanks for your feedback.

Luca.

Edwin Kempin

unread,
Apr 17, 2020, 7:30:06 AM4/17/20
to lucamilanesio, Repo and Gerrit Discussion
On Fri, Apr 17, 2020 at 1:22 PM lucamilanesio <luca.mi...@gmail.com> wrote:
Hi Checks plugin developers,
we have on the Gerrit CI checkers for the backend (Build/Test) and the PolyGerrit UI (PolyGerrit UI Test), however, not every change needs to perform both.

For example [1] is a PolyGerrit UI only change and therefore we don't perform the backend tests, because there were no changes on the backend.
Is there any way to do this "negative logic" for the Checker? (E.g. Do not trigger this Checker if condition XYZ)

AFAIK the only thing that checkers can do at the moment is associating to a Gerrit change query (see [2]) which may not give the expected result.
Why would it not give the expected result?

If a change query is not enough. You can over-match and then do additional filtering on the CI side (and then set the state to NOT_RELEVANT if the CI wants to skip it)
 
--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/76598919-19dc-4510-af82-bb984e45479d%40googlegroups.com.

Luca Milanesio

unread,
Apr 17, 2020, 7:39:47 AM4/17/20
to Edwin Kempin, Luca Milanesio, Repo and Gerrit Discussion

On 17 Apr 2020, at 12:29, Edwin Kempin <eke...@google.com> wrote:



On Fri, Apr 17, 2020 at 1:22 PM lucamilanesio <luca.mi...@gmail.com> wrote:
Hi Checks plugin developers,
we have on the Gerrit CI checkers for the backend (Build/Test) and the PolyGerrit UI (PolyGerrit UI Test), however, not every change needs to perform both.

For example [1] is a PolyGerrit UI only change and therefore we don't perform the backend tests, because there were no changes on the backend.
Is there any way to do this "negative logic" for the Checker? (E.g. Do not trigger this Checker if condition XYZ)

AFAIK the only thing that checkers can do at the moment is associating to a Gerrit change query (see [2]) which may not give the expected result.
Why would it not give the expected result?

If a change query is not enough. You can over-match and then do additional filtering on the CI side (and then set the state to NOT_RELEVANT if the CI wants to skip it)

Ah, good point, thanks for that.
I’ll use the NOT_RELEVANT check feedback then, thanks a lot, much appreciated :-)

Luca.

Alice Kober-Sotzek

unread,
Apr 20, 2020, 7:40:28 AM4/20/20
to Luca Milanesio, Edwin Kempin, Repo and Gerrit Discussion
On Fri, Apr 17, 2020 at 1:39 PM Luca Milanesio <luca.mi...@gmail.com> wrote:


On 17 Apr 2020, at 12:29, Edwin Kempin <eke...@google.com> wrote:



On Fri, Apr 17, 2020 at 1:22 PM lucamilanesio <luca.mi...@gmail.com> wrote:
Hi Checks plugin developers,
we have on the Gerrit CI checkers for the backend (Build/Test) and the PolyGerrit UI (PolyGerrit UI Test), however, not every change needs to perform both.

For example [1] is a PolyGerrit UI only change and therefore we don't perform the backend tests, because there were no changes on the backend.
Is there any way to do this "negative logic" for the Checker? (E.g. Do not trigger this Checker if condition XYZ)

AFAIK the only thing that checkers can do at the moment is associating to a Gerrit change query (see [2]) which may not give the expected result.
Why would it not give the expected result?

If a change query is not enough. You can over-match and then do additional filtering on the CI side (and then set the state to NOT_RELEVANT if the CI wants to skip it)

Ah, good point, thanks for that.
I’ll use the NOT_RELEVANT check feedback then, thanks a lot, much appreciated :-)
The NOT_RELEVANT state exists exactly for this purpose: whenever the change query can't be exact enough, CI systems can report NOT_RELEVANT when their additional logic determines that they don't need to actually run. If possible, though, the change queries should be refined to avoid unnecessary round trips with CI systems.

If our backend tests always wouldn't run when the frontend tests run, we could simply try negating parts of the query (e.g. adding "-directory:"polygerrit-ui"" to the backend tests). This would mean, though, that changes which touch both frontend and backend code would only have the frontend tests run and I guess that's not what we actually want. Luca, which logic is the CI system currently using to decide whether the backend tests have to run? Could we maybe translate that into a change query?

Luca.

 

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/76598919-19dc-4510-af82-bb984e45479d%40googlegroups.com.

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.

Luca Milanesio

unread,
Apr 20, 2020, 9:08:55 AM4/20/20
to Alice Kober-Sotzek, Luca Milanesio, Edwin Kempin, Repo and Gerrit Discussion

On 20 Apr 2020, at 12:40, Alice Kober-Sotzek <ali...@google.com> wrote:

On Fri, Apr 17, 2020 at 1:39 PM Luca Milanesio <luca.mi...@gmail.com> wrote:


On 17 Apr 2020, at 12:29, Edwin Kempin <eke...@google.com> wrote:



On Fri, Apr 17, 2020 at 1:22 PM lucamilanesio <luca.mi...@gmail.com> wrote:
Hi Checks plugin developers,
we have on the Gerrit CI checkers for the backend (Build/Test) and the PolyGerrit UI (PolyGerrit UI Test), however, not every change needs to perform both.

For example [1] is a PolyGerrit UI only change and therefore we don't perform the backend tests, because there were no changes on the backend.
Is there any way to do this "negative logic" for the Checker? (E.g. Do not trigger this Checker if condition XYZ)

AFAIK the only thing that checkers can do at the moment is associating to a Gerrit change query (see [2]) which may not give the expected result.
Why would it not give the expected result?

If a change query is not enough. You can over-match and then do additional filtering on the CI side (and then set the state to NOT_RELEVANT if the CI wants to skip it)

Ah, good point, thanks for that.
I’ll use the NOT_RELEVANT check feedback then, thanks a lot, much appreciated :-)
The NOT_RELEVANT state exists exactly for this purpose: whenever the change query can't be exact enough, CI systems can report NOT_RELEVANT when their additional logic determines that they don't need to actually run. If possible, though, the change queries should be refined to avoid unnecessary round trips with CI systems.

If our backend tests always wouldn't run when the frontend tests run, we could simply try negating parts of the query (e.g. adding "-directory:"polygerrit-ui"" to the backend tests). This would mean, though, that changes which touch both frontend and backend code would only have the frontend tests run and I guess that's not what we actually want. Luca, which logic is the CI system currently using to decide whether the backend tests have to run? Could we maybe translate that into a change query?

It is encoded in the function “collectBuildModes()” at:

Luca.

Alice Kober-Sotzek

unread,
Apr 20, 2020, 9:51:51 AM4/20/20
to Luca Milanesio, Edwin Kempin, Repo and Gerrit Discussion
On Mon, Apr 20, 2020 at 3:08 PM Luca Milanesio <luca.mi...@gmail.com> wrote:
On 20 Apr 2020, at 12:40, Alice Kober-Sotzek <ali...@google.com> wrote:
On Fri, Apr 17, 2020 at 1:39 PM Luca Milanesio <luca.mi...@gmail.com> wrote:
On 17 Apr 2020, at 12:29, Edwin Kempin <eke...@google.com> wrote:
On Fri, Apr 17, 2020 at 1:22 PM lucamilanesio <luca.mi...@gmail.com> wrote:
Hi Checks plugin developers,
we have on the Gerrit CI checkers for the backend (Build/Test) and the PolyGerrit UI (PolyGerrit UI Test), however, not every change needs to perform both.

For example [1] is a PolyGerrit UI only change and therefore we don't perform the backend tests, because there were no changes on the backend.
Is there any way to do this "negative logic" for the Checker? (E.g. Do not trigger this Checker if condition XYZ)

AFAIK the only thing that checkers can do at the moment is associating to a Gerrit change query (see [2]) which may not give the expected result.
Why would it not give the expected result?

If a change query is not enough. You can over-match and then do additional filtering on the CI side (and then set the state to NOT_RELEVANT if the CI wants to skip it)

Ah, good point, thanks for that.
I’ll use the NOT_RELEVANT check feedback then, thanks a lot, much appreciated :-)
The NOT_RELEVANT state exists exactly for this purpose: whenever the change query can't be exact enough, CI systems can report NOT_RELEVANT when their additional logic determines that they don't need to actually run. If possible, though, the change queries should be refined to avoid unnecessary round trips with CI systems.

If our backend tests always wouldn't run when the frontend tests run, we could simply try negating parts of the query (e.g. adding "-directory:"polygerrit-ui"" to the backend tests). This would mean, though, that changes which touch both frontend and backend code would only have the frontend tests run and I guess that's not what we actually want. Luca, which logic is the CI system currently using to decide whether the backend tests have to run? Could we maybe translate that into a change query?

It is encoded in the function “collectBuildModes()” at:
I see. As long as we don't have a query parameter to express that at least one file is outside of a specified directory, I wouldn't know how to express that logic as a change query. The NOT_RELEVANT state seems to be the way to go in this case.
Reply all
Reply to author
Forward
0 new messages