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

Migrate unit tests to satysfi-test #144

Merged
merged 39 commits into from
Apr 7, 2024
Merged

Migrate unit tests to satysfi-test #144

merged 39 commits into from
Apr 7, 2024

Conversation

zeptometer
Copy link
Collaborator

@zeptometer zeptometer commented Feb 14, 2021

This PR migrates regression tests from generic.saty to unit tests with satysfi-test. This closes #142.

What this PR does

This PR introduces a new directory /test with test definitions. The top-level file main.test.saty loads all test sources. run-test.sh compiles the whole source in text-mode, and output report.txt, including test result. It runs in CI to ensure that satysfi-base works up to expectation, which you can find .github/workflows/ci.yml.

Why to migrate to satysfi-test

The regression tests from generic.saty make snapshots from compiler output, which is not very readable and maintainable. Unit tests with satysfi-test make tests organized, clear and maintainable. It also allows test-first development: It will be beneficial for future development.

@@ -6,6 +6,29 @@

type ordering = Lt | Gt | Eq

module Ordering : sig
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added eq for ordering. I'm a bit afraid that Ord and Ordering are confusing. Any suggestion?

src/tuple.satyg Outdated Show resolved Hide resolved

let promise-test-cases = open Test in
describe `promise module` [
it `evaluate lazily` (fun () -> (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. The original test uses Debug.log to check the order of side effects, but this is not feasible with the satisfy-test. Instead, this test case uses ref to make side effects.
  2. The original test has additional cases that I did not find helpful. That's why they are omitted in this test case.

test (seq (group (many1 char-a)) (group (many1 char-b))) `aabbb`
|> Expect.is-true
));
% TODO GH-142: Is this right description?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I try to make descriptions from test cases, but I need more confidence with some of them. Need help from @nyuichi

|> flleq [2.; 3.; 5.; neg 2.; neg 3.; neg 5.; 6.])
|> Expect.is-true
));
it `virtually test exp2i` (fun () -> (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What would be a good description? @nyuichi


let fn-test-cases = open Test in
describe `Fn module` [
it `make recursive function with fix` (fun x -> (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The original test case calls Debug.log for each recursion, but it is not feasible in satysfi-test. That's why I write this case in this way.

@zeptometer zeptometer force-pushed the zeptometer/GH-142 branch 3 times, most recently from 52e2268 to a526552 Compare March 5, 2024 08:01
@zeptometer zeptometer marked this pull request as ready for review March 5, 2024 08:23
@nyuichi nyuichi merged commit 37e6774 into master Apr 7, 2024
16 checks passed
@nyuichi nyuichi deleted the zeptometer/GH-142 branch April 7, 2024 06:46
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.

Try migrating unit tests to satysfi-test
2 participants