genodelabs / goa

Tool for streamlining the development of Genode applications
GNU Affero General Public License v3.0
19 stars 17 forks source link

goa: testing #43

Closed ssumpf closed 1 year ago

ssumpf commented 1 year ago

We want to add a simple testing facility to Goa in order to identify regressions quickly. The current suggestion is that there will be a script in <goa>/share/goa/lib that executes everything in the examples directory and compares the actual output to the expected output (using expect). Any objections or suggestions are welcome.

jschlatow commented 1 year ago

That is a reasonable suggestions. I'd only add a couple of feature requests and ideas regarding the script:

nfeske commented 1 year ago

@jschlatow whereas I like very much where you want to go, the motivation behind this concrete issue does not come from the Goa-feature perspective but from the quality-assurance needs of the Goa tool itself.

I think it would be sensible to start with only testing the examples (as a concrete regression test) and keep automated testing as an actual feature of Goa for later. The expect script will give us the peace of mind that the basic steps described in the guidance documentation are working as intended, and issues like #30 can no longer happen.

jschlatow commented 1 year ago

@nfeske Thanks for the clarification about the scope of this issue.

ssumpf commented 1 year ago

@jschlatow, @nfeske: Okay, e21de60 is my first take on the simple test script. Since it is indeed very small, we can change everything very quickly until everyone is alright with it.

ssumpf commented 1 year ago

I am trying to build a small Qt5 test as well, but they are all huge and always need the GUI (which might not work on headless systems).

cproc commented 1 year ago

I am trying to build a small Qt5 test as well, but they are all huge and always need the GUI (which might not work on headless systems).

There are two text-only tests (qt_core and qt_core_cmake) in the libports repository, if that helps?

jschlatow commented 1 year ago

@ssumpf I think the script serves its purpose well.

At first, I was a bit confused by the commit message because it mentions the artifacts (note the plural 's') file and comparing the first line of it against the output. Looking at the code, however, clarified that the goa project under test is executed until the first line of the test/artifact file or the timeout occurs. Could you clarify the commit message accordingly?

ssumpf commented 1 year ago

@jschlatow: I have changed the commit message and spell checked comments in d04d2f0

ssumpf commented 1 year ago

@jschlatow: 9eed12c adds a simple Qt5 example while ef16cc7 deletes the correct directories after a test run.

nfeske commented 1 year ago

@ssumpf you already went some steps into the direction that @jschlatow was assuming, but I'd be more cautious. For now, please let us maintain the list of tests along with the success patterns inside the test script, not adding a new formalism (test list and artifact file). This way, the examples stay clean of things that would call for documentation/explanation. The examples should stay as as concise as possible, answering questions instead of raising ones.

Considering the path down the road, specifying success patterns is already covered by the pkg/runtime files as used by the depot_autopilot. So the most natural way to add automated testing as a Goa feature (as a later step) would be to merely evaluate these conditions as done by the test.run script (see https://github.com/genodelabs/genode/blob/master/repos/os/run/test.run). So we can reuse an existing convention instead of creating a new one. That said, before taking this step for Goa, I'd reconsider the current success pattern definition of the depot_autopilot to make it a bit simpler. (I'll create an issue for that in the genode repository).

Once Goa supports the evaluation of the success criterion specified in pkg/runtime files, all that's left needed is the convention to host automated tests at /pkg/test/runtime. So a goa run test in a top-level directory would execute the tests in all sub directories. Wouldn't that be nice? But as I said, I'd not rush it.

As a minor remark, test.tcl may better be named test.expect because it is an expect script.

nfeske commented 1 year ago

I'll create an issue for that in the genode repository

As promised: https://github.com/genodelabs/genode/issues/4922

ssumpf commented 1 year ago

766092e is a version without test/artifact and test.list.

ssumpf commented 1 year ago

Once you agreed on https://github.com/genodelabs/genode/issues/4922, I can have a look.

ssumpf commented 1 year ago

@jschlatow: 6fe78e5 implements your suggestion.

nfeske commented 1 year ago

https://github.com/genodelabs/goa/commit/766092e8d0e26adecb926be68d666cce6d691140 is a version without test/artifact and test.list

Cool, thank you.

jschlatow commented 1 year ago

Excellent! I merged the three commits to staging.

jschlatow commented 1 year ago

@ssumpf I'm closing this issue because the commits entered the master branch. We can open a separate issue regarding an automated-testing feature for Goa in due course.

ssumpf commented 1 year ago

@jschlatow: The Qt5 example is broken. 17d1141 fixes it.

ssumpf commented 1 year ago

@jschlatow: I think we should run the test script after every update to staging.

jschlatow commented 11 months ago

@jschlatow: I think we should run the test script after every update to staging.

Agreed. Thanks for the fix, I just merged it to staging.