Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Test runner #88

Merged
merged 6 commits into from Jul 27, 2022
Merged

Test runner #88

merged 6 commits into from Jul 27, 2022

Conversation

IcyTv
Copy link
Contributor

@IcyTv IcyTv commented Jul 25, 2022

The current test runner relies on a built version of mica to run its tests and on a bash-like shell. This is suboptimal since rust already provides a good framework, which also allows for parallel testing.

To make use of this, I added a custom rust test runner, making use of libtest-mimic to emulate the base behavior of cargo test.

This test runner loads all mica files from the tests folder and then runs them as actual tests with output, etc. It also identifies each test by using it's containing folder.

To enable skipping of long tests (like with stress-1 or 2) it is possible to add a .skip extension to stop them from running by default and only run them when using cargo test -- --ignored.

It is also possible to add basic exception testing. The example is failing.ml, in which we assert a false value and then check that the output contains the text "assertion failed". This is currently still pretty limited, but it would definitively be possible to expand this (i.e. by using .err files or sth. to compare more complex error outputs)

The current test runner relies on a built version of mica to run its tests
and on a bash-like shell
This is suboptimal since rust already provides a good framework

To make use of this, I added a custom rust test runner,
making use of libtest-mimic to emulate the base behaviour
of cargo test.

This test runner loads all mica files from the tests folder
and then runs them as actual tests
Now the test runner identifies each test by its containing folder
Copy link
Member

@liquidev liquidev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the pull request! I didn't know about libtest-mimic's existence. It's nice that you included support for failing tests, this is something I wanted to add myself but improving the test runner wasn't a priority.

Outside of this review, I see a couple things that could be improved:

  • It seems that your editor or some other tool reformatted Cargo.toml, which messed up the ordering of keys in the [package] section and changed some whitespace formatting (3 spaces → 2 spaces, no spaces after { and before }.) It would be nice if you could fix that.
  • If we have a new runner, the old bash-based runner should be removed. The README for tests should reflect this change and include the appropriate command that's needed to run language tests.
  • The CI build workflow still makes use of the old test runner.
  • It seems that while testing out skipping functionality, you forgot to reenable GC stress tests. These tests must run to ensure that the garbage collector can deal with arbitrarily large reference chains, and are important because if the GC doesn't work properly we lose memory safety.

tests/runner.rs Outdated Show resolved Hide resolved
tests/runner.rs Outdated Show resolved Hide resolved
tests/runner.rs Outdated Show resolved Hide resolved
tests/runner.rs Outdated Show resolved Hide resolved
The general idea is: All tests must succeed always.
The `.skip`attribute is intended to skip long tests by default
@IcyTv
Copy link
Contributor Author

IcyTv commented Jul 27, 2022

The idea behind skipped/ignored tests is to disable long running tests. This can speed up development while still keeping the integrity of the language. In my eyes Stress tests sound painful and shouldn't need to be run every time, but then again I don't know enough about the language to say... If you think it's a good idea to have them enabled by default, I can fix that as well :^)

@liquidev
Copy link
Member

Alright, I understand the idea better now. As long as normally-skipped tests are run by CI, I don't think there should be a problem.

@liquidev liquidev merged commit e9764ce into mica-lang:master Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants