Skip to content

Import mspec tests to run our spec code#2508

Draft
herwinw wants to merge 27 commits into
natalie-lang:masterfrom
herwinw:mspec_tests
Draft

Import mspec tests to run our spec code#2508
herwinw wants to merge 27 commits into
natalie-lang:masterfrom
herwinw:mspec_tests

Conversation

@herwinw
Copy link
Copy Markdown
Member

@herwinw herwinw commented Jan 17, 2025

After the recent fixes in #2496, where we were using the wrong method to run a certain spec, I noticed that the upstream mspec repo contained specs for mspec. The code uses rspec to run the mspec specs (yes, this sentence reads terrible).

This is a very early attempt to import these specs and run our spec runner through it. For now, it just runs a single test, and it needs a wrapper that changes like 90% of the test itself to make it compatible.

I will try some more things, but this might just end up with me abandoning it.

@herwinw herwinw self-assigned this Jan 17, 2025
@herwinw herwinw force-pushed the mspec_tests branch 5 times, most recently from 0bf93a7 to e9c38cd Compare January 18, 2025 09:18
@herwinw
Copy link
Copy Markdown
Member Author

herwinw commented Jan 18, 2025

I guess the intention of this change has shifted a bit when I was working on it. Since the internal interface of our specs runner is incompatible with the upstream one, just trying to import everything is a bad idea. Instead, I just started with importing the specs for a single matcher, and refactored the code until we could use the upstream matcher. Then it turned into a change to import the upstream mspec code.
The end goal is to slowly replace our spec runner with the upstream mspec code.

@herwinw herwinw force-pushed the mspec_tests branch 2 times, most recently from bef3f20 to 7e7ca93 Compare January 18, 2025 12:48
@herwinw
Copy link
Copy Markdown
Member Author

herwinw commented Mar 20, 2025

The specs for the matrix gem include a custom matcher. I updated that one in the checkout to match our spec runner, but the nightly tests use a clean specs checkout and those still fail. So I guess I've found some motivation to finish this.

herwinw added 16 commits May 19, 2025 19:14
This is a verbatim copy from upstream (commit
0aabb3e548eb5ea6cad0125f8f46cee34542b6b7) and should be used to test our
spec runner, to prevent issues like the instance_of/kind_of mixup we
recently fixed.

Of course, this means we need to be able to run it first.

Eventually, we should be able to run all the tests, but let's just start
with a single one.
This is very much in the PoC state
This Matcher now can be used in the mspec tests, the expectatin is used
when running the regular specs. The matcher now fully satisfies the
upstream specs, whilst the behaviour of the expectation is unchanged
(except for an improved error message).
We can now run the original file, this will help with the remaining
specs.
And update the stub mspec/matchers file to point to our matchers list.
Make an Expectation class instead, that can run any matcher. The
remaining code is generic enough to be completely disjointed from the
actual matcher.

The generic class is in a weird code spot, but putting it on the top of
the list actually caused git to complete screw up the diff. This way it
stays readable, and we should probably just fix the other two the same
way.
We now have a MSpecMatchers module, and we can just add another stub
file to load our spec helper.
Import MSpec.format as well, and alter our MSpec definition to be a
module instead of a class.
We ended up with copying both the code and the specs from the mspec
repository, so we were effectively running their specs on their code.
There's not much point on doing that, so get rid of those specs again.
@herwinw herwinw force-pushed the mspec_tests branch 2 times, most recently from ea1f37d to f639bde Compare May 19, 2025 18:00
herwinw added 2 commits May 19, 2025 20:12
Keep our code for now, we're not fully compatible with the mspec
version.
Keep our code for now, we're not fully compatible with the mspec
version.
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.

1 participant