exercism / rust-test-runner

GNU Affero General Public License v3.0
3 stars 15 forks source link

Upgrade to version 2 spec #17

Closed ErikSchierboom closed 4 weeks ago

ErikSchierboom commented 3 years ago

We've recently updated the test runner interface specification to allow for two types of test runners, identified via a new version property in the results.json file (see the spec for the version property):

  1. version: 1 test runners are quite basic, and only detect if either all tests passed or not. These test runners capture the console output of the test runner and output that. They do not include details on individual tests passing/failing
  2. version: 2 test runners contain detailed information on individual tests passing/failing. We've recently added a required field named test_code, which contains the code the test ran to verify the behavior (see the spec for the test_code property).

The test runner's output is currently what I refer to as version 1.5: it contains the individual test information, but does not yet contain the test_code key. If possible, the test runner should be updated to also include the test code, in which case the version property should also be added with 2 as its value.

You are completely free on how you want to extract the test code, either via the AST or via finding the test code in the source code text via string manipulation.

Let me know if there are any questions.

senekor commented 9 months ago

I think this has to be done with AST parsing. Some test files use modules to organize the tests, I believe there is even one exercise where test cases are generated with a declarative macro.

The spec also says something about returning test results in the same order as the tests are defined. I believe this is currently not done correctly. Having AST information about the test file would be the perfect preparation to fix that as well.

senekor commented 1 month ago

I did a bit of research. The canoncial way to parse Rust code into an AST (syn) does not resolve macro invocations, which makes sense. That means we will definitely have to change the tests of some exercises so they don't contain custom macros anymore, or at least not ones that are difficult to read for Rust learners. There are only five exercises which use macros in their tests left anyway.

I'm currently also thinking that string manipulation shouldn't be too difficult to implement. Once the macros are gone, there might not be a big advantage to go the AST route anymore.

senekor commented 1 month ago

blocked on https://github.com/exercism/rust/issues/1824

senekor commented 1 month ago

While exploring how to implement this, I realized that some exercises have more test files than just the normal tests/<slug>.rs. Actually there are just two such exercises: forth and macros. It's probably fine to merge the extra test file of forth into the normal. The special alloc-attack test defines its own allocator, but it shouldn't interfere with the other tests.

The macros exercise however is more difficult. These tests need to ensure that some pieces of code don't compile. So the tests contain essentially other cargo projects.

The spec says that the test code SHOULD be included for practice exercises - so it's not required. I think the best approach is just to not include the test code for the macros exercise.

ErikSchierboom commented 1 month ago

The spec says that the test code SHOULD be included for practice exercises - so it's not required. I think the best approach is just to not include the test code for the macros exercise.

I think that's fine.