Qual: Improve hurl script, add to pre-commit, add to win-ci #36279

mdeweerd · 2025-11-16T15:33:03Z

Qual: Improve hurl script, add to pre-commit, add to win-ci

Improvements to hurl script (get API key, logging for github actions), make independent from PWD;
Add hook to pre-commit (manual run, is a reminder on how to run it, facilitates running it);
Add hurl run to win-ci (enabled through parameter in workflow).

mdeweerd · 2025-11-16T17:16:03Z

@JonBendtsen I've added hurl to the windows flow. It's only executed on forks and manual workflow triggers at this time.

See https://github.com/mdeweerd/dolibarr/actions/runs/19409056135 for some results.

I updated logToCs to extract the hurl errors.

JonBendtsen · 2025-11-16T17:48:59Z

@JonBendtsen I've added hurl to the windows flow. It's only executed on forks and manual workflow triggers at this time.

See https://github.com/mdeweerd/dolibarr/actions/runs/19409056135 for some results.

I updated logToCs to extract the hurl errors.

Great idea, though right now something has changed such that the hurl tests that used to work no longer works :-(

Eventually I wish that hurl (or something similar) would be part of the build process so we would detect possible issues earlier

I even see it as useful for Dolibarr administrators because if they run a hurl test to check their setup and detect changes between upgrades and/or configuration changes then they can better tell their users of the changes with a new version.

JonB

mdeweerd · 2025-11-16T20:19:32Z

When tests are not in ci, they end up no longer working.
The same is valid for the static analyzers. As long as errors were ignored in files, more of the ignored notices were added.

Now run.sh can run just a few tests and I added a pre-commit hook configuration that runs only changed hurl files (pre-commit run --hook-stage manual hurl) - however the file need to be added to run the latest version of the hurl script.

ONe reason for failure in my case seem to be that debug information is added - so hurl would need to ignore debug information somehow:

Success api/setup/10_setup_company.hurl (1 request(s) in 1138 ms)
error: Assert body value
   --> api/warehouses/10_warehouses.hurl:42:1
    |
    | GET http://{{hostnport}}/api/index.php/warehouses/0
    | ...
 42 | {"error":{"code":400,"message":"Bad Request: No warehouse with id=0 can exist"}}
    |   -{"error":{"code":400,"message":"Bad Request: No warehouse with id=0 can exist"}}
    |   +{
    |   +    "error": {
    |   +        "code": 400,
    |   +        "message": "Bad Request: No warehouse with id=0 can exist"
    |   +    },
    |   +    "debug": {
    |   +        "source": "api_warehouses.class.php:79 at call stage",
    |   +        "stages": {
    |   +            "success": [
    |   +                "get",
    |   +                "route",
    |   +                "negotiate",
    |   +                "authenticate",
    |   +                "validate"
    |   +            ],
    |   +            "failure": [
    |   +                "call",
    |   +                "message"
    |   +            ]
    |   +        }
    |   +    }
    |   +}
    |

test/hurl/README.md

JonBendtsen · 2025-11-16T20:35:35Z

When tests are not in ci, they end up no longer working. The same is valid for the static analyzers. As long as errors were ignored in files, more of the ignored notices were added.

yeah, I totally agree, I was just not yet ready to do it my self, and I also did not expect that it would break so fast.

I hoped - and this might be what happened ;-) - that someone with more Dolibarr/development/PHP experience than me would take the initiative and put it into the CI :-D

Some of the things hold me back was that I do not know if the build system starts a Dolibarr, what data it contains, which modules are enabled, ... so I feared ending up enabling something which would always fail - and that is not useful either.

Now run.sh can run just a few tests and I added a pre-commit hook configuration that runs only changed hurl files (pre-commit run --hook-stage manual hurl) - however the file need to be added to run the latest version of the hurl script.

Looks great :-)

ONe reason for failure in my case seem to be that debug information is added - so hurl would need to ignore debug information somehow:

OR ASSERT that there would be debug information ;-)

Success api/setup/10_setup_company.hurl (1 request(s) in 1138 ms)
error: Assert body value
   --> api/warehouses/10_warehouses.hurl:42:1
    |
    | GET http://{{hostnport}}/api/index.php/warehouses/0
    | ...
 42 | {"error":{"code":400,"message":"Bad Request: No warehouse with id=0 can exist"}}
    |   -{"error":{"code":400,"message":"Bad Request: No warehouse with id=0 can exist"}}
    |   +{
    |   +    "error": {
    |   +        "code": 400,
    |   +        "message": "Bad Request: No warehouse with id=0 can exist"
    |   +    },
    |   +    "debug": {
    |   +        "source": "api_warehouses.class.php:79 at call stage",
    |   +        "stages": {
    |   +            "success": [
    |   +                "get",
    |   +                "route",
    |   +                "negotiate",
    |   +                "authenticate",
    |   +                "validate"
    |   +            ],
    |   +            "failure": [
    |   +                "call",
    |   +                "message"
    |   +            ]
    |   +        }
    |   +    }
    |   +}
    |

JonBendtsen · 2025-11-16T20:39:25Z

Now run.sh can run just a few tests and I added a pre-commit hook configuration that runs only changed hurl files (pre-commit run --hook-stage manual hurl) - however the file need to be added to run the latest version of the hurl script.

What if there are no changed hurl files, only changes in non hurl files? should hurl ideally be run at each and every commit? just like all the other checks?

mdeweerd · 2025-11-16T20:55:59Z

I don't know if we should keep the numbering system

I just refactored this part of the documentation.
Numbering is one approach - the run script needs to ensure these script are run in the correct order and preferably directory by directory - but no need for now to force this.

Some of the things hold me back was that I do not know if the build system starts a Dolibarr, what data it contains, which modules are enabled, ...

Enabling & disabling a module is something to think about - or maybe already ok. However, no example so far I think.

What if there are no changed hurl files, only changes in non hurl files? should hurl ideally be run at each and every commit? just like all the other checks?

When the tests are short, they can run on every commit. If not, the script could be smarter in the future and examine which files are commited, what test they belong to and run these.
I added a 'hurl-all' hook to run all the test. However, both are in the "manual" stage so not triggered automatically - just for testing, documentation and readiness.

OR ASSERT that there would be debug information ;-)

I think this is one of the things that the tests must be hardened for - debug information is not always present, so it should ignore this.
Or, disable it when running the tests.
The debug information changes because the line information changes.

JonBendtsen · 2025-11-16T21:09:18Z

I don't know if we should keep the numbering system

I just refactored this part of the documentation. Numbering is one approach - the run script needs to ensure these script are run in the correct order and preferably directory by directory - but no need for now to force this.

yeah, that was my thought, such that POSTS to put in data which was then GET, PUT and finally DELETE. For this order matters and numbers are one way of getting that.

Some of the things hold me back was that I do not know if the build system starts a Dolibarr, what data it contains, which modules are enabled, ...

Enabling & disabling a module is something to think about - or maybe already ok. However, no example so far I think.

What if there are no changed hurl files, only changes in non hurl files? should hurl ideally be run at each and every commit? just like all the other checks?

When the tests are short, they can run on every commit. If not, the script could be smarter in the future and examine which files are commited, what test they belong to and run these. I added a 'hurl-all' hook to run all the test. However, both are in the "manual" stage so not triggered automatically - just for testing, documentation and readiness.

yeah, and we could possible have many smaller files, and some kind of system to detect which function in which file was changed, because there is no need to test unchanged functions. I see some methods for doing this:

In the hurl files have a line identifying which file and function they are testing, then look for those files
In the PHP file have a line identifying which hurl tests should be run
Simply naming the hurl files matching the corresponding file - and possibly function - that they are testing.

Like

api/compta/facture/api_invoices.post.hurl
api/comm/mailing/api_mailings.index.hurl
...

OR ASSERT that there would be debug information ;-)

I think this is one of the things that the tests must be hardened for - debug information is not always present, so it should ignore this. Or, disable it when running the tests. The debug information changes because the line information changes.

Given that the error message is json, right even with the debug information, then I think that rather than folding this down to one line

{
 "error": {
   "code": 400,
   "message": "Bad Request: Combo of Source=externasssl and Type=LEAD not found in dictionary with active invoice contact types"
 }
}

then you could possibly use something like

[Asserts]
jsonpath "$.error" exists
jsonpath "$.error.code" == 400
jsonpath "$.error.message" == "Bad Request: Combo of Source=externasssl and Type=LEAD not found in dictionary with active invoice contact types"

Let's try that.

JonBendtsen · 2025-11-16T21:38:45Z

OR ASSERT that there would be debug information ;-)

I think this is one of the things that the tests must be hardened for - debug information is not always present, so it should ignore this. Or, disable it when running the tests. The debug information changes because the line information changes.

Given that the error message is json, right even with the debug information, then I think that rather than folding this down to one line
{
 "error": {
   "code": 400,
   "message": "Bad Request: Combo of Source=externasssl and Type=LEAD not found in dictionary with active invoice contact types"
 }
}
then you could possibly use something like
[Asserts]
jsonpath "$.error" exists
jsonpath "$.error.code" == 400
jsonpath "$.error.message" == "Bad Request: Combo of Source=externasssl and Type=LEAD not found in dictionary with active invoice contact types"
Let's try that.

I think this commit can handle both if there is a "debug" key in the json and if there is not

bd01176

mdeweerd · 2025-11-16T21:43:21Z

I'll let you evolve the tests - you can test the updated script.

I've added the exclusion option, and also a help option.

JonBendtsen · 2025-11-16T21:49:39Z

I'll let you evolve the tests - you can test the updated script.

If I am the only one creating/writing/fixing/updating/... tests then it will take a while before everything is covered.

Preferably I would want the Dolibarr community to get to the point that if a PR that adds new functionality doesn't come with a working hurl test, then it wouldn't get merged - Possibly we need to have someone help writing the tests, but they are not that hard, and often one can find an example to copy.

I do not think that my Dolibarr gives the debug output that you got in the output you included above.

mdeweerd · 2025-11-16T22:02:24Z

If I am the only one creating/writing/fixing/updating/... tests then it will take a while before everything is covered.

Open-source is generally though there. I decided to help with improving the flow, but not so much with the functionnality. I fixed >20k phan notices, implemented a fast script to check for missing translations keys, setup the win-ci flow, ... . And I have some other opensource projects to attend to.

Preferably I would want the Dolibarr community to get to the point that if a PR that adds new functionality doesn't come with a working hurl test, then it wouldn't get merged - Possibly we need to have someone help writing the tests, but they are not that hard, and often one can find an example to copy.

I am sure there will be some momentum at some point. More examples and even some slide for eldy to present can be helpful.

I do not think that my Dolibarr gives the debug output that you got in the output you included above.

It's also in https://github.com/mdeweerd/dolibarr/actions/runs/19411735332/job/55534085760 and the database is built from scratch using dev/setup/phpunit/setup_conf.sh which is used in wndows-ci and close to what is in travis.yml too.

JonBendtsen · 2025-11-16T22:24:55Z

If I am the only one creating/writing/fixing/updating/... tests then it will take a while before everything is covered.

Open-source is generally though there. I decided to help with improving the flow, but not so much with the functionnality. I fixed >20k phan notices, implemented a fast script to check for missing translations keys, setup the win-ci flow, ... . And I have some other opensource projects to attend to.

yeah I can imagine it could be tough, volunteers and such. Dolibarr is so far the only project I contribute to.

Preferably I would want the Dolibarr community to get to the point that if a PR that adds new functionality doesn't come with a working hurl test, then it wouldn't get merged - Possibly we need to have someone help writing the tests, but they are not that hard, and often one can find an example to copy.

I am sure there will be some momentum at some point. More examples and even some slide for eldy to present can be helpful.

yeah maybe I should write such a slide - or possibly show up and being able to answer questions and tell about my experience - I just don't speak french, but I do plan to talk and share at the International DevCamp.

In the https://github.com/Dolibarr/dolibarr/blob/develop/test/hurl/README.md file I did write that we (Dolibarr community) should discuss here in 2025, but I haven't seen any, and I also did not write where we should have that discussion. Do you know of any place where such a discussion has happened?

I do not think that my Dolibarr gives the debug output that you got in the output you included above.

It's also in https://github.com/mdeweerd/dolibarr/actions/runs/19411735332/job/55534085760 and the database is built from scratch using dev/setup/phpunit/setup_conf.sh which is used in wndows-ci and close to what is in travis.yml too.

okay. So far I just used hurl tests with my own development Dolibarr with a database restored from my production Dolibarr.

JonBendtsen · 2025-11-16T22:27:39Z

I do not think that my Dolibarr gives the debug output that you got in the output you included above.

It's also in https://github.com/mdeweerd/dolibarr/actions/runs/19411735332/job/55534085760 and the database is built from scratch using dev/setup/phpunit/setup_conf.sh which is used in wndows-ci and close to what is in travis.yml too.

I think that your GUI tests fails because it is not logged in.

mdeweerd · 2025-11-17T00:13:22Z

I think that your GUI tests fails because it is not logged in.

I think this is better:
https://github.com/mdeweerd/dolibarr/actions/runs/19414098187/job/55539703596

mdeweerd · 2025-11-17T00:34:15Z

FYI, some resources

Last Intl devcamp - https://dolibarr.de/herbst-doliday-2025
https://www.youtube.com/@DolibarrOrg/playlists

JonBendtsen · 2025-11-17T06:55:58Z

FYI, some resources

Last Intl devcamp - https://dolibarr.de/herbst-doliday-2025

I was in Vienna

https://www.youtube.com/@DolibarrOrg/playlists

it's hard when the presentations are in French

JonBendtsen · 2025-11-17T06:59:47Z

I think that your GUI tests fails because it is not logged in.

I think this is better: https://github.com/mdeweerd/dolibarr/actions/runs/19414098187/job/55539703596

My Browser counts 31 times with the string: Enter login details so I think that the GUI doesn't login yet.

mdeweerd · 2025-11-17T09:09:06Z

API test that uses API key created from login/password is working - so I suppose API key is ok:
The ones that fail next are expected error codes but they have a debug field in the json reply.

Notice: 2.a. Running API tests that do require authentication
Success api/setup/10_setup_company.hurl (1 request(s) in 91 ms)

Then we have:

Success gui/save_login_cookie.hurl (2 request(s) in 684 ms)

And failing tests next - so it seems to be an issue with the cookie file.

mdeweerd · 2025-11-17T13:45:07Z

I found a reason: the windows version of hurl needs a windows like path.
I managed to fine a method to set it.

"Show logo on menu" is not in the output (not even in a regular browser when logged in), tests still fail, but at least now the cookie is ok.

However, it is clear that the login cookie is not ok.
We get a "DOLSESSTIMEOUT" in ci, but I get "DOLSESSID_*" locally.
I am doing another run with maximum verbosity when getting the login cookie.

https://github.com/mdeweerd/dolibarr/actions

2025-11-17T13:26:51.4176367Z COOKIE CONTENTS 2 for C:\TEMP\hurl_cookie19431130081.jar
2025-11-17T13:26:51.4280739Z # Netscape HTTP Cookie File
2025-11-17T13:26:51.4281652Z # This file was generated by Hurl
2025-11-17T13:26:51.4282282Z 
2025-11-17T13:26:51.4284607Z # Cookies for file <gui\save_login_cookie.hurl>
2025-11-17T13:26:51.4285879Z #HttpOnly_127.0.0.1	FALSE	/	FALSE	0	DOLSESSTIMEOUT_3c631c8b8e034871c4eb0e3723b10e8988db98dd	2000

mdeweerd · 2025-11-17T14:18:53Z

Locally I get a correct cookie.
I also tried with a simple PHP server, php7.4 and php8.2 - both work locally (erase the cookiejar file before the test
hurl --very-verbose --variable 'hostnport=doli' --variable 'username=admin' --secret 'password=admin' --test test/hurl/gui/save_login_cookie.hurl --cookie-jar 'C:\TEMP\hurl_cookie19431638776.jar'
or
hurl --very-verbose --variable "hostnport=doli" --variable "username=admin" --secret "password=admin" --test test/hurl/gui/save_login_cookie.hurl --cookie-jar "C:\TEMP\hurl_cookie19431638776.jar"

Quite strange that the behavior is different in ci - the credentials are ok because I can get an API token with it and locally I used the same procedure for the database contents (sql available in the WIN CI artifact by the way).

php -S 127.0.0.1:8000 -t htdocs

# php.ini
[php]
open_basedir="D:\workspace\documents;D:\workspace\htdocs"

JonBendtsen · 2025-11-17T20:04:22Z

"Show logo on menu" is not in the output (not even in a regular browser when logged in), tests still fail, but at least now the cookie is ok.

maybe you can try with this test method #36294 which uses a different text string

mdeweerd · 2025-11-17T22:40:48Z

"Show logo on menu" is not in the output (not even in a regular browser when logged in), tests still fail, but at least now the cookie is ok.

maybe you can try with this test method #36294 which uses a different text string

I'll check when it's integrated.

For the moment the issue seems elsewhere: CSRF. Not sure that this should be invalidating the cookie, so
Locally the tests work with php as a webserver, so it can work with that.

From dolibarr.log (available in the win-ci run artifacts):

2025-11-17 22:23:35 WARNING 127.0.0.1          6128        --- Access to POST /index.php refused by CSRF protection (invalid token), so we disable POST and some GET parameters - referrer=, action=, _GET|POST['token']=12dc17cb8248bd647a76f38a127c17a7
2025-11-17 22:23:35 DEBUG   127.0.0.1          6128        checkLoginPassEntity usertotest=admin entitytotest=1 authmode=dolibarr

Not so good IMHO but not blocking I suppose.

2025-11-17 22:23:36 WARNING 127.0.0.1          6128        functions::dol_include_once Tried to load unexisting file: /core/boxes/box_members.php
2025-11-17 22:23:36 WARNING 127.0.0.1          6128        Failed to load box 'box_members' into file '/core/boxes/box_members.php'

- Add HURL test runner to Windows CI workflow - Include environment variables for test configuration - Update test execution step to run HURL tests when enabled

- Added --verbose and --very-verbose options to control hurl output verbosity - Updated help message to include new options - Modified test execution to use the VERBOSE variable - Added shellcheck disable for SC2086 to handle the VERBOSE variable correctly

mdeweerd force-pushed the qual/hurl/win branch 2 times, most recently from 149096e to 53d2ece Compare November 16, 2025 17:14

mdeweerd marked this pull request as ready for review November 16, 2025 17:16

mdeweerd force-pushed the qual/hurl/win branch from c86fb8f to 92e1deb Compare November 16, 2025 20:07

JonBendtsen reviewed Nov 16, 2025

View reviewed changes

test/hurl/README.md Show resolved Hide resolved

mdeweerd force-pushed the qual/hurl/win branch from 92e1deb to 088b680 Compare November 16, 2025 20:33

JonBendtsen mentioned this pull request Nov 16, 2025

Improve hurl test script with arguments #36273

Closed

mdeweerd force-pushed the qual/hurl/win branch 3 times, most recently from 747f309 to 4a625b1 Compare November 16, 2025 23:57

mdeweerd force-pushed the qual/hurl/win branch from c73632a to 558ded5 Compare November 17, 2025 10:46

mdeweerd marked this pull request as draft November 17, 2025 10:59

mdeweerd force-pushed the qual/hurl/win branch from 3d5b504 to 499b898 Compare November 17, 2025 11:58

mdeweerd mentioned this pull request Nov 17, 2025

Qual: Enhance hurl test documentation and run script #36291

Merged

mdeweerd force-pushed the qual/hurl/win branch from 062d819 to 2242248 Compare November 17, 2025 22:12

mdeweerd force-pushed the qual/hurl/win branch from 2242248 to 0d60ec9 Compare November 19, 2025 17:19

JonBendtsen mentioned this pull request Nov 19, 2025

Qual/Fix: remove entity=2 hurl test which doesn't work #36336

Merged

mdeweerd added 5 commits November 20, 2025 14:21

Qual: Add new pre-commit hook for running hurl tests (manual trigger)

a3e9bb1

QUAL: Add HURL test integration for Windows CI

d24c0ef

- Add HURL test runner to Windows CI workflow - Include environment variables for test configuration - Update test execution step to run HURL tests when enabled

🎨 Modification to debug hurl cookie

1945f28

very verbose hurl for login cookie

4564c14

mdeweerd force-pushed the qual/hurl/win branch from 0d60ec9 to 4564c14 Compare November 20, 2025 13:22

Uh oh!

Qual: Improve hurl script, add to pre-commit, add to win-ci #36279

Are you sure you want to change the base?

Qual: Improve hurl script, add to pre-commit, add to win-ci #36279

Conversation

mdeweerd commented Nov 16, 2025

Qual: Improve hurl script, add to pre-commit, add to win-ci

Uh oh!

mdeweerd commented Nov 16, 2025

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

mdeweerd commented Nov 16, 2025

Uh oh!

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

mdeweerd commented Nov 16, 2025

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

mdeweerd commented Nov 16, 2025

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

mdeweerd commented Nov 16, 2025

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

JonBendtsen commented Nov 16, 2025

Uh oh!

mdeweerd commented Nov 17, 2025

Uh oh!

mdeweerd commented Nov 17, 2025

Uh oh!

JonBendtsen commented Nov 17, 2025

Uh oh!

JonBendtsen commented Nov 17, 2025

Uh oh!

mdeweerd commented Nov 17, 2025

Uh oh!

mdeweerd commented Nov 17, 2025

Uh oh!

mdeweerd commented Nov 17, 2025

Uh oh!

JonBendtsen commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdeweerd commented Nov 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JonBendtsen commented Nov 17, 2025 •

edited

Loading