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

Add tests for not (monadic ~) #90

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add tests for not (monadic ~) #90

wants to merge 7 commits into from

Conversation

pmikkelsen
Copy link
Collaborator

No description provided.

@pmikkelsen
Copy link
Collaborator Author

This pull request adds tests for monadic ~ (not), but it also refactors _RunVariations_ so it runs the model function, instead of the caller. In my opinion, this is the only correct thing to do, as the plan is to add more and more random "variations" of the tests to that operator, which requires that it can run the model and compare the result.

It also means it now becomes possible to pass data to _RunVariations_ which results in errors being generated from the primitive and model function. The errors are compared, just like non-errors are compared.

This pull-request will break all other tests because they use _RunVariations_ the old way, where the right argument contained the expected data as well, often computed as such ... RunVariations (d1 model d2) d1 d2. The fix there is simply to remove (d1 model d2), as it will be computed automatically. For monadic calls such as ... RunVariations (model data) data, the fix is to change it to ... RunVariations ⊂data.

A lot of tests pass explicitly computed "expected values" to _RunVariations_, which is in my opinion better handled by calls to Assert. RunVariations is not able to do anything meaningful with a model function, if the expected values are pre-computed.

@pmikkelsen
Copy link
Collaborator Author

I see that auto-format in RIDE has caused all the changes to become a bit difficult to see. I apologise for that.

@pmikkelsen
Copy link
Collaborator Author

The tests introduced in this pull-request requires the fix for mantis 21762 to be merged into the interpreter.

@pmikkelsen pmikkelsen marked this pull request as ready for review January 17, 2025 09:27
@pmikkelsen pmikkelsen requested a review from sloorush January 17, 2025 09:27
@sloorush
Copy link
Member

Thanks @pmikkelsen

When would the fix for mantis 21762 be merged into the interpreter?

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.

2 participants