Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[RFC] Proposal for integrated test suite in Alire #1831

Open
AldanTanneo opened this issue Feb 3, 2025 · 9 comments
Open

[RFC] Proposal for integrated test suite in Alire #1831

AldanTanneo opened this issue Feb 3, 2025 · 9 comments

Comments

@AldanTanneo
Copy link
Contributor

AldanTanneo commented Feb 3, 2025

Disclaimer: this is a very rough draft of a feature, in order to collect more thoughts and ideas and eventually implement something useful for the community at large.

The goal is to provide a very simple test functionality in Alire, explicitly targeted towards small community crates. That means: very little setup, NO extra dependencies.

The default test runner would be easily replaceable, however, to allow bigger projects to use more complex test drivers.

CLI changes

  • Retire current alr test subcommand, rename it alr index-test (breaking change; Alire 3.0?)
  • Use the alr test command for running actual crate tests
  • alr test --generate: create basic file structure for tests with default test runner. This is run only once, when setting up the project.

Manifest changes

  • New section: [tests]

Default keys:

runner = "alire" # or custom runner command, for ex:
                 # runner = "./run.sh"
                 # runner = ["testsuite.py", "<extra args>"]

location = "tests" # run tests command in this sub-folder
  • The test runner command is executed in the alire execution environment; that means it can use binary dependencies specified in the tests sub-folder's alire.toml, if it exists (recommended). This opens the possibility to provide more advanced test runners in the alire index.

Default test runner

Goals: be extremely simple, but also very easy to replace

Default dir structure (tests is a sub-folder of the main crate folder):

tests
|_ src
|  |_ tests-[name1].adb
|  |_ tests-[name2].adb
|_ common
|  |_ tests.ads
|  |_ [test helper files]
|_ alire.toml
|_ tests.gpr

All tests are sub-procedures of a main Tests package, optionally with test helper code located in common. Each test file contains a single procedure that will be run in its own process (potentially in parallel - leverage process isolation).

Default alire.toml and tests.gpr are created by alr test --generate; the GPR file imports a config file specifying the Main targets, that is recreated when the tests to compile and run change (ie. from test filtering or from adding/removing tests). When running alr test, those main files are compiled and executed.

Sub-crate's alire.toml can contain test only dependencies, environment overrides, etc.

A test is considered PASSING when its exit code is 0, FAILING otherwise. This catches assertion fails (if they return non-zero exit codes, like in AUnit) and unhandled exceptions. (Maybe provide default Assert functions that exit with a non-zero code? or simply reuse AUnit's Assertions -> leave choice to the user with alire.toml test dependencies)

Other possibilities:

  • test timeout in [tests] section
  • .adb file prelude in TOML format specifying custom test settings? (goes against simplicity goal? [TO DISCUSS])
@Fabien-Chouteau
Copy link
Member

Thanks @AldanTanneo ,

The thing I liked the most when we first discussed your idea is that it's both extremely simple to start writing tests, but also quite open in terms of how you want to implement the tests. You can just use pragma Assert, or go to a library like Aunit, trendy_test, etc.

@LionelDraghi
Copy link
Contributor

I agree on the alr test renaming.

When we have both Ada unit tests and external tools to run, does it mean that we would have to write a single top "run.sh"? And how will it cope with the Ada part?
Maybe would it be simpler (for the user, not for Alire devs :-) to have a list of runner in the tests section.

Also, it may be interesting at initialization to test if there is a classical Makefile with a "check" target in the root crate's dir, and to propose it in the runner list...
(It seems doable with "make -q check")

@AldanTanneo
Copy link
Contributor Author

AldanTanneo commented Feb 4, 2025

When we have both Ada unit tests and external tools to run, does it mean that we would have to write a single top "run.sh"? And how will it cope with the Ada part?
Maybe would it be simpler (for the user, not for Alire devs :-) to have a list of runner in the tests section.

That's an interesting idea! The proposed [tests] section could become [[tests]] and it would be a way to add several successive commands in the test process. There could even be something like 'steps' names, and doing alr test --step=$STEP_NAME would only run that one?

Alternatively, one could use the tests subcrate's "build actions" to add more commands to the tests. I think the current Alire "test actions" are supposed to emulate that in a way, but imo it's a tad confusing, with the current alr test behaviour that tests an entire index...

Also, it may be interesting at initialization to test if there is a classical Makefile with a "check" target in the root crate's dir, and to propose it in the runner list...

I'm a bit reluctant to add a dependency on make existing to do this. It expects a very specific Makefile setup and shouldn't be a silent default (maybe with explicit confirmation if a Makefile is detected?). It also seems a bit redundant with the actions/steps idea.

@mosteo
Copy link
Member

mosteo commented Feb 4, 2025

Wrt. to renaming alr test, currently alr test by default does test the current crate. It does so by building it, unless a test action has been defined. If the switches to test remote crates in the index are confusing they can be moved, yes.

If you define test actions (https://github.com/alire-project/alire/blob/master/doc/catalog-format-spec.md#release-information), they will be run in order instead of the default build. This is already used in our index submissions. If necessary, we could redefine the fields in the test action (or add e.g. a timeout); or we could move test actions to their own [[test]] array, if that seems clearer.

In my unstated plans about testing improvements, when a new crate is initialized, a default test action should exist. This way users know that a modicum of testing is happening, and that they can add likewise other, more sophisticated tests.

This default test action, at least before we have something more fancy like this proposal, should run a nested crate, that should also exist by default (no need to alr test --generate). The nested crate makes explicit what is happening now: it builds the crate (and additionally, if it is a library, links an executable against it (as currently we are not catching problems with linker options due to no executable being generated)).

That would be superseded by a more test-oriented structure like the one proposed here, of course. This was the bare minimum I wanted to have in the short term.

I'm currently experimenting with templating so maintaining and improving generation of crates is simpler; that will certainly help with this proposal too.

@AldanTanneo
Copy link
Contributor Author

Thanks for the detailed explanations on the current behaviour of alr test, it makes it a bit clearer (at least to me.) I think providing a hands-on, little-to-no setup way to write and run tests would still be very valuable as the crate index grows and more people want to write libraries.

As a newcomer to the Ada ecosystem, my experience is that test actions are confusing and there is no clear, simple way to do testing in Alire. I feel like a minimal default test runner helps to alleviate that issue. And indeed, a templated crate is what I intended for the tests subfolder, except it would have many Main procedures instead of just one.

@mosteo
Copy link
Member

mosteo commented Feb 4, 2025

Yes, I agree that having something like this is necessary. And you're right that at the moment there's nothing about testing in the manifest, and even if you find the guidelines in the documentation, it is just 'have a nested crate do the testing'.

@krischik
Copy link

krischik commented Feb 5, 2025

I'm a bit worried this would be a breaking change to the tests I already have in AdaCL as I don't use a generated tests but wrote my tests with AUnit by hand.

Note that currently all applications in ./test*/ are executed with alr run and I make use of that. So the used of any directory with starting with "test" could potentially affected.

@AldanTanneo
Copy link
Contributor Author

AldanTanneo commented Feb 5, 2025

If this proposal goes forward, the goal is to make it very configurable.

There would be a breaking change in the manifest file, if we go with the [[tests]] section, but then you could just set your runner to be ["alr", "run"] and have the same behaviour as previously.

Alternatively, there could be no default behaviour in an empty manifest, but crate creation could set up a default [[tests]] section with a runner = "alire" entry, to be changed by people needing more complex test runners. I kind of personally prefer having a default behaviour, in a "no setup" way, as it avoids bloating the manifest for simple crates; but I can understand it being undesirable when an ecosystem already exists. It all depends on what stability guarantees alire wants to offer at this stage. Is it released, stable software? Or are breaks acceptable because it's still considered experimental?

I do wish there was some kind of manifest versioning, so that breaks could be avoided altogether between alire versions.

@mosteo
Copy link
Member

mosteo commented Feb 6, 2025

I do wish there was some kind of manifest versioning, so that breaks could be avoided altogether between alire versions.

Manifests follow index syntax, so they're versioned in a way. alr version tells which index versions are understood. The shortcoming is that no index version is stated in the manifest itself, and the differences between versions aren't well documented in a central place.

But, when we introduce breaking or incremental changes in the index/manifest syntax, this is known by the next alr and it will warn about using an old or incompatible index. I don't even think we have ever made breaking changes to local manifests since the beginnings. There have been additions (like [[tests]] would be) but not incompatibilities, at least not in recent memory. (Here I consider a breaking change when a newer alr is unable to use an older manifest. When an older alr cannot use a newer manifest is what I call incremental; you need only update alr but need not touch your manifests. And newer alr come with a matching index so that should never be a problem.)

Running with --force allows alr to ignore unknown entries in the manifest, which may be helpful sporadically (e.g. you want to try some crate that is not in your old community index), but it is not intended as a general solution. Then again, I don't think people should work with older alrs relative to the index or manifests they're using, as you cannot guarantee that things will work properly when you discard information.

As for local manifests, I see value in having their explicit version in them; this would allow explicit identification of mismatches and in a way would help to identify old manifests. Expecting that alr adjust its parsing according to versions exceeds my expectations at the moment, and wouldn't help anyway for unknown versions of a manifest (older alr than manifest).

Note that currently all applications in ./test*/ are executed with alr run and I make use of that. So the used of any directory with starting with "test" could potentially affected.

Actually this was intended as a temporary workaround for a while since at the time alr test was not adequate. The idea nowadays is that you explicitly define your tests using [[action]], and during PR checks we just run alr test. And, in fact, the behavior you @krischik describe is no longer in effect (we do not look for /test* folders) since alire-project/alire-index-checks@50ee869

TL;DR: the proposal here is an improvement over our current situation, but existing test actions already allow what is proposed here if we generated the test crate during alr init.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants