Bug 39877 - CI - Incremental runs
Summary: CI - Incremental runs
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 39876
Blocks:
  Show dependency treegraph
 
Reported: 2025-05-12 13:40 UTC by Jonathan Druart
Modified: 2025-05-13 12:51 UTC (History)
1 user (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 39877: Incremental test runs (12.90 KB, patch)
2025-05-12 20:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Remove files that have been deleted (1.08 KB, patch)
2025-05-12 20:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Merge vue and js tidy tests (2.62 KB, patch)
2025-05-12 20:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Apply to other slow tests (5.15 KB, patch)
2025-05-12 20:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Incremental test runs (13.30 KB, patch)
2025-05-13 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Remove files that have been deleted (1.08 KB, patch)
2025-05-13 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Merge vue and js tidy tests (2.62 KB, patch)
2025-05-13 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Apply to other slow tests (5.15 KB, patch)
2025-05-13 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Incremental test runs (13.33 KB, patch)
2025-05-13 12:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Remove files that have been deleted (1.08 KB, patch)
2025-05-13 12:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Merge vue and js tidy tests (2.62 KB, patch)
2025-05-13 12:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39877: Apply to other slow tests (5.15 KB, patch)
2025-05-13 12:32 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-05-12 13:40:49 UTC
The current (full) test suite runs in 2h20 (Koha_Main/3253 on Docker_15).

We should improve that.

One of the main reason is the recent introduction of new xt tests that are run on all files (valid, tidy, pod, codespell, etc).


From a previous run (last week) I noted this top 5:

xt/vue_tidy.t          105446 ms
xt/perltidy.t          175970 ms
xt/js_tidy.t           239615 ms
xt/author/codespell.t  467041 ms
xt/pl_valid.t          2502354 ms
Comment 1 Jonathan Druart 2025-05-12 20:35:53 UTC
Created attachment 182308 [details] [review]
Bug 39877: Incremental test runs

The idea of this patchset is to allow increment runs for some of our
tests.
The current (full) test suite runs in 2h20 (Koha_Main/3253 on Docker_15).

One of the main reason is the recent introduction of new xt tests that are run on all files (valid, tidy, pod, codespell, etc).

From a previous run (last week) I noted this top 5:

xt/vue_tidy.t          105446 ms
xt/perltidy.t          175970 ms
xt/js_tidy.t           239615 ms
xt/author/codespell.t  467041 ms
xt/pl_valid.t          2502354 ms

We are going to merge js_tidy.t and vue_tidy.t as it does no longer make
sense to have a difference between them (we had at the beginning of the
vue work, because of differences in the options, now everything is
handled in the .prettierrc config file)

Once this is pushed, we will be able to create a new project on gitlab
and pass some env variables from Jenkins.
Suggestion: project would be named https://gitlab.com/koha-community/koha-ci-results
When everything will be setup, jenkins will run the tests one the whole
test suite, then the next run will be "incremental": we are going to run
the tests on the files that were failing and the files modified by the
last push.
Example:
 * "commit abc": all the files are tested, xt/perltidy.t is failing on Koha.pm
 * koha-ci-results/perltidy/abc is created and contains ['Koha.pm']
 * "commit def" has a modification on about.pl
 * xt/perltidy.t is checking tidy on Koha.pm and about.pl, both are tidy
 * koha-ci-results/perltidy/def is created and contains []

Test plan:
* Create an empty project on gitlab
  eg. https://gitlab.com/me/koha-ci-results
* Generate an access token with the developer role
  (https://gitlab.com/me/koha-ci-results/-/settings/access_tokens)
  Copy the token (in the following noted as TOKEN_REPLACE_ME)
* Unprotect the "main" branch
* Run the following prove command:
```
KOHA_CI_INCREMENTAL_RUN_REPO_URL=gitlab.com/me/koha-ci-results.git \
KOHA_CI_INCREMENTAL_RUNS=1 \
KOHA_CI_INCREMENTAL_RUNS_REPORT=1 \
KOHA_CI_INCREMENTAL_RUNS_TOKEN=TOKEN_REPLACE_ME \
prove xt/perltidy.t
```
=> a new commit is created on your repo
* Add some mess in one of your perl file, commit using `--no-verify` to
  bypass the git hook
* Run the prove command again
=> only the file you have modified is tested
=> a new commit is created and the file is reported as failing
* Add another commit, fix the failure, add other mess
=> confirm that what you are seeing makes sense :)

* Ideally redo with the different tests modified by this patchset

Note that you can also export the following env vars to set git author
detail:
 * GIT_COMMITTER_NAME
 * GIT_COMMITTER_EMAIL
 * GIT_AUTHOR_NAME
 * GIT_AUTHOR_EMAIL

Question: What are we going to use on Jenkins?
Comment 2 Jonathan Druart 2025-05-12 20:35:56 UTC
Created attachment 182309 [details] [review]
Bug 39877: Remove files that have been deleted

Ignore deleted or renamed files
Comment 3 Jonathan Druart 2025-05-12 20:35:58 UTC
Created attachment 182310 [details] [review]
Bug 39877: Merge vue and js tidy tests
Comment 4 Jonathan Druart 2025-05-12 20:36:05 UTC
Created attachment 182311 [details] [review]
Bug 39877: Apply to other slow tests
Comment 5 Jonathan Druart 2025-05-12 20:40:20 UTC
Discussion for QA: How should we handle concurrency? It's crucial to prevent multiple tests from pushing simultaneously. To minimize the risk of the repository becoming outdated, we might need to perform a pull right before each push. Is that enough? Keep in mind that only one job will be run at the same time (Koha_Main).

Maybe later we will also use it for stable branches.
Comment 6 Jonathan Druart 2025-05-12 20:42:42 UTC
An important note I forgot in the commit message. You can use the incremental run without the report (the report will be only for Jenkins).
So it will help RM as xt tests will run (way) faster.
Comment 7 Jonathan Druart 2025-05-12 21:06:24 UTC
Note that we may miss some bad bugs. For instance if a version of codespell changes and that catch more typo, we will miss them with the incremental run even if the test fails in a regular run. Could also happen with pl_valid I guess if an external lib introduces changes that breaks our code. But they really are edge cases we cannot avoid and that should be caught by the qa script anyway.
Comment 8 Jonathan Druart 2025-05-13 10:11:44 UTC
Created attachment 182347 [details] [review]
Bug 39877: Incremental test runs

The idea of this patchset is to allow increment runs for some of our
tests.
The current (full) test suite runs in 2h20 (Koha_Main/3253 on Docker_15).

One of the main reason is the recent introduction of new xt tests that are run on all files (valid, tidy, pod, codespell, etc).

From a previous run (last week) I noted this top 5:

xt/vue_tidy.t          105446 ms
xt/perltidy.t          175970 ms
xt/js_tidy.t           239615 ms
xt/author/codespell.t  467041 ms
xt/pl_valid.t          2502354 ms

We are going to merge js_tidy.t and vue_tidy.t as it does no longer make
sense to have a difference between them (we had at the beginning of the
vue work, because of differences in the options, now everything is
handled in the .prettierrc config file)

Once this is pushed, we will be able to create a new project on gitlab
and pass some env variables from Jenkins.
Suggestion: project would be named https://gitlab.com/koha-community/koha-ci-results
When everything will be setup, jenkins will run the tests one the whole
test suite, then the next run will be "incremental": we are going to run
the tests on the files that were failing and the files modified by the
last push.
Example:
 * "commit abc": all the files are tested, xt/perltidy.t is failing on Koha.pm
 * koha-ci-results/perltidy/abc is created and contains ['Koha.pm']
 * "commit def" has a modification on about.pl
 * xt/perltidy.t is checking tidy on Koha.pm and about.pl, both are tidy
 * koha-ci-results/perltidy/def is created and contains []

Test plan:
* Create an empty project on gitlab
  eg. https://gitlab.com/me/koha-ci-results
* Generate an access token with the developer role
  (https://gitlab.com/me/koha-ci-results/-/settings/access_tokens)
  Copy the token (in the following noted as TOKEN_REPLACE_ME)
* Unprotect the "main" branch
* Run the following prove command:
```
KOHA_CI_INCREMENTAL_RUN_REPO_URL=gitlab.com/me/koha-ci-results.git \
KOHA_CI_INCREMENTAL_RUNS=1 \
KOHA_CI_INCREMENTAL_RUNS_REPORT=1 \
KOHA_CI_INCREMENTAL_RUNS_TOKEN=TOKEN_REPLACE_ME \
prove xt/perltidy.t
```
=> a new commit is created on your repo
* Add some mess in one of your perl file, commit using `--no-verify` to
  bypass the git hook
* Run the prove command again
=> only the file you have modified is tested
=> a new commit is created and the file is reported as failing
* Add another commit, fix the failure, add other mess
=> confirm that what you are seeing makes sense :)

* Ideally redo with the different tests modified by this patchset

* Note that you can use the incremental run without the report (the report will
be only for Jenkins).
It will help RM as xt tests will run (way) faster.

Note that you can also export the following env vars to set git author
detail:
 * GIT_COMMITTER_NAME
 * GIT_COMMITTER_EMAIL
 * GIT_AUTHOR_NAME
 * GIT_AUTHOR_EMAIL

Question: What are we going to use on Jenkins?

Discussion for QA: How should we handle concurrency?
It's crucial to prevent multiple tests from pushing simultaneously.
To minimize the risk of the repository becoming outdated, we might need to perform
a pull right before each push. Is that enough? Keep in mind that only one job will
be run at the same time (Koha_Main).
Maybe later we will also use it for stable branches.
Comment 9 Jonathan Druart 2025-05-13 10:11:47 UTC
Created attachment 182348 [details] [review]
Bug 39877: Remove files that have been deleted

Ignore deleted or renamed files
Comment 10 Jonathan Druart 2025-05-13 10:11:50 UTC
Created attachment 182349 [details] [review]
Bug 39877: Merge vue and js tidy tests
Comment 11 Jonathan Druart 2025-05-13 10:11:53 UTC
Created attachment 182350 [details] [review]
Bug 39877: Apply to other slow tests
Comment 12 Jonathan Druart 2025-05-13 12:32:30 UTC
Created attachment 182355 [details] [review]
Bug 39877: Incremental test runs

The idea of this patchset is to allow increment runs for some of our
tests.
The current (full) test suite runs in 2h20 (Koha_Main/3253 on Docker_15).

One of the main reason is the recent introduction of new xt tests that are run on all files (valid, tidy, pod, codespell, etc).

From a previous run (last week) I noted this top 5:

xt/vue_tidy.t          105446 ms
xt/perltidy.t          175970 ms
xt/js_tidy.t           239615 ms
xt/author/codespell.t  467041 ms
xt/pl_valid.t          2502354 ms

We are going to merge js_tidy.t and vue_tidy.t as it does no longer make
sense to have a difference between them (we had at the beginning of the
vue work, because of differences in the options, now everything is
handled in the .prettierrc config file)

Once this is pushed, we will be able to create a new project on gitlab
and pass some env variables from Jenkins.
Suggestion: project would be named https://gitlab.com/koha-community/koha-ci-results
When everything will be setup, jenkins will run the tests one the whole
test suite, then the next run will be "incremental": we are going to run
the tests on the files that were failing and the files modified by the
last push.
Example:
 * "commit abc": all the files are tested, xt/perltidy.t is failing on Koha.pm
 * koha-ci-results/perltidy/abc is created and contains ['Koha.pm']
 * "commit def" has a modification on about.pl
 * xt/perltidy.t is checking tidy on Koha.pm and about.pl, both are tidy
 * koha-ci-results/perltidy/def is created and contains []

Test plan:
* Create an empty project on gitlab
  eg. https://gitlab.com/me/koha-ci-results
* Generate an access token with the developer role
  (https://gitlab.com/me/koha-ci-results/-/settings/access_tokens)
  Copy the token (in the following noted as TOKEN_REPLACE_ME)
* Unprotect the "main" branch
* Run the following prove command:
```
KOHA_CI_INCREMENTAL_RUN_REPO_URL=gitlab.com/me/koha-ci-results.git \
KOHA_CI_INCREMENTAL_RUNS=1 \
KOHA_CI_INCREMENTAL_RUNS_REPORT=1 \
KOHA_CI_INCREMENTAL_RUNS_TOKEN=TOKEN_REPLACE_ME \
prove xt/perltidy.t
```
=> a new commit is created on your repo
* Add some mess in one of your perl file, commit using `--no-verify` to
  bypass the git hook
* Run the prove command again
=> only the file you have modified is tested
=> a new commit is created and the file is reported as failing
* Add another commit, fix the failure, add other mess
=> confirm that what you are seeing makes sense :)

* Ideally redo with the different tests modified by this patchset

* Note that you can use the incremental run without the report (the report will
be only for Jenkins).
It will help RM as xt tests will run (way) faster.

Note that you can also export the following env vars to set git author
detail:
 * GIT_COMMITTER_NAME
 * GIT_COMMITTER_EMAIL
 * GIT_AUTHOR_NAME
 * GIT_AUTHOR_EMAIL

Question: What are we going to use on Jenkins?

Discussion for QA: How should we handle concurrency?
It's crucial to prevent multiple tests from pushing simultaneously.
To minimize the risk of the repository becoming outdated, we might need to perform
a pull right before each push. Is that enough? Keep in mind that only one job will
be run at the same time (Koha_Main).
Maybe later we will also use it for stable branches.
Comment 13 Jonathan Druart 2025-05-13 12:32:33 UTC
Created attachment 182356 [details] [review]
Bug 39877: Remove files that have been deleted

Ignore deleted or renamed files
Comment 14 Jonathan Druart 2025-05-13 12:32:35 UTC
Created attachment 182357 [details] [review]
Bug 39877: Merge vue and js tidy tests
Comment 15 Jonathan Druart 2025-05-13 12:32:38 UTC
Created attachment 182358 [details] [review]
Bug 39877: Apply to other slow tests
Comment 16 Julian Maurice 2025-05-13 12:39:01 UTC
> xt/pl_valid.t          2502354 ms
We can also save some time by not checking *.t files, since they are executed anyway. *.t files represent approximately 25% of the total time of pl_valid.t for me.

(In reply to Jonathan Druart from comment #7)
> Could also happen with pl_valid I
> guess if an external lib introduces changes that breaks our code. But they
> really are edge cases we cannot avoid and that should be caught by the qa
> script anyway.
If an exported subroutine is removed/renamed, code that still use/import it will fail to compile, but this won't get caught if we are only checking modified files. Will it be caught by the QA script ?
Comment 17 Jonathan Druart 2025-05-13 12:51:04 UTC
(In reply to Julian Maurice from comment #16)
> > xt/pl_valid.t          2502354 ms
> We can also save some time by not checking *.t files, since they are
> executed anyway. *.t files represent approximately 25% of the total time of
> pl_valid.t for me.

Could be, yes.

> (In reply to Jonathan Druart from comment #7)
> > Could also happen with pl_valid I
> > guess if an external lib introduces changes that breaks our code. But they
> > really are edge cases we cannot avoid and that should be caught by the qa
> > script anyway.
> If an exported subroutine is removed/renamed, code that still use/import it
> will fail to compile, but this won't get caught if we are only checking
> modified files. Will it be caught by the QA script ?

I have no idea why I have written that, it does not make sense.