Test Module #259

Foxinio · 2025-10-23T15:14:31Z

In this PR I propose module to write structured tests.
(This implementation is inspired by one done by @wojpok, and included in #256)

Tests can be written like so:

let suite = createTestSuite "Prelude/*"

let _ = suite.addTest "`chr` calls on error" (fn () =>
  assertCallsOnError (fn _ => chr 300))

let _ = suite.addTest "`chr` does not call on error" (fn () =>
  assertDoesNotCallOnError (fn _ => chr 127))

# Clearly not correct implementation
let mkStr _ _ = ""

let _ = suite.addTest "`mkStr` count = 0" (fn _ =>
  assertEq (mkStr "abc" 0) "")

let _ = suite.addTest "`mkStr` count < 0" (fn _ =>
    assertEq (mkStr "abc" (-1)) "";
    assertEq (mkStr "abc" (-10)) "")
  
let _ = suite.addTest "`mkStr` count > 0" (fn _ =>
    assertEq (mkStr "abc" 1) "abc";
    assertEq (mkStr "abc" 2) "abcabc";
    assertEq (mkStr "abc" 3) "abcabcabc";
    assertEq (mkStr "abc" 5) "abcabcabcabcabc")

Tests are run whenever file containing tests is included, in one passed to dbl.
This may seem undesirable, but will be addressed with introduction of attributes.

wojpok · 2025-10-28T08:28:57Z

I highly suggest the following style of writing test suites:

let _ =
  let addTest = craeteTestSuite "mySuite" >.addTest in
    addTest "test1" (fn .....);
    addTest "test"2 (fn .....)

That way it will be much easier to introduce @{test ...} attributes

Foxinio · 2025-10-28T08:46:16Z

I highly suggest the following style of writing test suites:
let _ =
  let addTest = craeteTestSuite "mySuite" >.addTest in
    addTest "test1" (fn .....);
    addTest "test"2 (fn .....)
That way it will be much easier to introduce @{test ...} attributes

This would work, if suites were flat structures. But since we made them tree-like, allowing suites to contain other suites, this will not work.

wojpok · 2025-10-30T09:47:39Z

I still consider nested test suites to be a bloat. We dont have fixtures, so there really is not need to complicate this mechanism. Also what difference does make currentSuite.addSuite "nested suite" to directly creating new suite createTestSuite "nested suite"? They still execute in sequence.

Foxinio · 2025-10-30T13:34:46Z

I prefer this solution because of its scalability. While current implementation doesn't really make use of these structures being nested, this module is rich in avenues for extension. So:

User interface isn't significantly impacted by this being nested structures.
Implementation would barely change to go back to flat structures, and as person implementing it I can say, previously it was more complicated.
There are multiple ways this can be extended like:

Adding suite prefixes, so you can later filter tests by them.
Adding tags to tests and suites, and make tests inherit those tags.

While it is possible to achieve grouping of tests, when using flat structure, it would be cumbersome and you would have to invent a convention and then keep to it without being automatically checked if you really follow this convention. Ultimately this is not necessary when you could have grouping mechanism built into framework at no real cost.

Foxinio · 2025-10-30T13:37:39Z

I also no see that I misunderstood your earlier comment.
I have no problem with introducing your convention, and it may even look nicer.
But I have to emphasize, it doesn't give us any computational benefits.

wojpok

From the functional standpoint, the code is fine. I don't like some of the design decisions. I don't get the idea of global tests and recursive suites. There should be just regular suites and nothing more. Let's discuss this with rest of the team during the next meeting.

lib/AnsiTerminal.fram

wojpok · 2025-10-31T14:11:25Z

lib/Test.fram

+  , errorType : ErrorType
+  }
+
+{# TODO : Add tags to tests and suites #}


Don't. Tags will handled using attributes as already agreed upon

I don't believe we did. From what i remember we discussed what attributes should be, but at the time we didn't consider how this would change once there is a proper testing module in place, and we considered that there could be changes.
As to why i think this would be advantageous, firstly having it expressed inside language gives use much more flexibility, secondly it allows for having a far more fine-grained tags.
I can imagine having a test file with

let tags = ...

defined at the top, and being able to pass just this named parameter, instead of having to write it out every time.

However this seems to be another thing to be discussed at the meeting.

wojpok · 2025-10-31T14:15:13Z

lib/Test.fram

+  , body : Unit ->[IO] List ErrorInfo
+  }
+
+data rec TestSuiteInternal =


Neither TestSuiteInternal nor TestSuite are public. User can't interact with them directly, hence why the redundancy?

The second one is abstr, as it should be usable outside the module.
TestSuiteInternal shouldn't be visible outside this module and so the handle is returned.

Abstr makes type visible outside module. User can't interact with this structure unless public function/method is provided, just like you already did. You can move all data from TestSuiteInternal to abstract TestSuite and user will have no idea that there are any additional data. The most delicate part of this structure are references that are in fact already shared. There is no immediate benefit to this structure.

lib/Test.fram

wojpok · 2025-10-31T14:21:59Z

lib/Test.fram

+method printTestSuccess (TestCase { name, line, file }) =
+  printStr   "[ \{ Color.setFgColor Color.Green | strFmt }OK";
+  printStr   "\{Color.resetAll | strFmt} ]";
+  printStrLn " \{ name | strFmt} from \{ file | strFmt }:\{ line | strFmt }"


I don't think there is a need to include test definition location. It looks ugly honestly

To me it looks fine, i guess something else to vote on :D.

wojpok · 2025-10-31T14:24:45Z

lib/Test.fram

+##}
+data TestArbiter E =
+  { abort : {X} -> ErrorInfo ->[E] X
+  , fail  : ErrorInfo ->[E] Unit


fail is confusing. It does not indicate that contrary to abort the test is resumed upon calling fail.

I couldn't find a better name for saying that if condition isn't met, the test is resumed but is considered failed.
Also in my opinion the type says it very well, fail always returns unit, contrary to abort which never returns.

Fair enough. Consider adding a comment that explains the difference. That will do for me

lib/Test.fram

wojpok · 2025-10-31T14:30:27Z

lib/Test.fram

+  let tests = ioMut.ref [] in
+  let suite' = TestSuiteInternal { name, file, line, tests, suites } in
+  suite.addSuite suite';
+  TestSuite { suites, tests }


Reference to recursive suites and test list is shared. Why are you using different 2 objects here?

As answered above, this is the place where this is used.

Foxinio

Addressed most of the comments, all that's left is to conform this module to what we agree on in the meeting.

Foxinio · 2025-10-31T19:37:28Z

lib/Test.fram

+  , body : Unit ->[IO] List ErrorInfo
+  }
+
+data rec TestSuiteInternal =


The second one is abstr, as it should be usable outside the module.
TestSuiteInternal shouldn't be visible outside this module and so the handle is returned.

Foxinio · 2025-10-31T19:38:02Z

lib/Test.fram

+  let tests = ioMut.ref [] in
+  let suite' = TestSuiteInternal { name, file, line, tests, suites } in
+  suite.addSuite suite';
+  TestSuite { suites, tests }


As answered above, this is the place where this is used.

Foxinio · 2025-10-31T19:38:53Z

lib/Test.fram

+method printTestSuccess (TestCase { name, line, file }) =
+  printStr   "[ \{ Color.setFgColor Color.Green | strFmt }OK";
+  printStr   "\{Color.resetAll | strFmt} ]";
+  printStrLn " \{ name | strFmt} from \{ file | strFmt }:\{ line | strFmt }"


To me it looks fine, i guess something else to vote on :D.

Foxinio · 2025-11-01T14:03:54Z

lib/Test.fram

+  , errorType : ErrorType
+  }
+
+{# TODO : Add tags to tests and suites #}


I don't believe we did. From what i remember we discussed what attributes should be, but at the time we didn't consider how this would change once there is a proper testing module in place, and we considered that there could be changes.
As to why i think this would be advantageous, firstly having it expressed inside language gives use much more flexibility, secondly it allows for having a far more fine-grained tags.
I can imagine having a test file with

let tags = ...

defined at the top, and being able to pass just this named parameter, instead of having to write it out every time.

However this seems to be another thing to be discussed at the meeting.

Foxinio · 2025-11-02T17:21:11Z

lib/Test.fram

+data rec TestSuiteInternal =
+  { name   : String
+  , line   : Int
+  , file   : String
+  , suites : Ref IO (List TestSuiteInternal)
+  , tests  : Ref IO (List TestCase)
+  }
+
+abstr data TestSuite =
+  { suites : Ref IO (List TestSuiteInternal)
+  , tests  : Ref IO (List TestCase)
+  }


This is meant to be two part. One is the internal representation, the other is a public facing handle to access test module interface. Probably this could be reduced to one abstract type, and the semantics should be preserved. But i don't see any harm in having both, and separating them based on the use result's in much clearer code.

wojpok · 2025-11-03T21:50:29Z

I have made up my mind. I am fine with this code as is. It would be nice to merge TestSuiteInternal and TestSuite to single structure, but I won't enforce that. Only issue at hand is lack of any mechanism to toggle test excecution. If you provide a suitable way of controlling it I will approve this PR

Foxinio · 2025-11-04T07:43:04Z

I agree, it has to be done, and so I left a TODO comment. However I think it's best to merge it in this state and improve in a later PR, as this change would require a significant amount of code to be written.

In my opinion, for now, we can live with all included tests being run.

wojpok · 2025-11-04T08:47:33Z

Not necessarily. What about a global flag that can be read using extern and would be controlled via designated flag in command line? A neat temporary solution that might be substituted in the future easily

wojpok

Code is okay, all issues are resolved. I am not a fan of the command line output, but I don't really want to slow down the development. We need proper testing environment sooner than later

Foxinio · 2025-12-15T11:49:53Z

Work developed in this PR is to be continued in #289.

Foxinio added 6 commits October 20, 2025 17:21

Testing module partly finished

1917c1e

Minor problems remain

44bc6f9

Most problems fixed, addressing comments

a42aa14

Ready for presentation

ca73c33

Test Module finished and ready for review

4008277

Removing commented out code from AnsiTerminal

0f04299

wojpok reviewed Oct 31, 2025

View reviewed changes

Foxinio added 2 commits October 31, 2025 20:30

Addressing comments

0ae2900

Minor changes

b7e0787

Foxinio commented Nov 2, 2025

View reviewed changes

Addressing comments and general discussion

b7c9261

wojpok approved these changes Nov 17, 2025

View reviewed changes

My finishing touches

8199061

Foxinio closed this Dec 15, 2025

Foxinio deleted the testing branch December 15, 2025 11:50

Test Module #259

Test Module #259

Uh oh!

Conversation

Foxinio commented Oct 23, 2025

Uh oh!

wojpok commented Oct 28, 2025

Uh oh!

Foxinio commented Oct 28, 2025

Uh oh!

wojpok commented Oct 30, 2025

Uh oh!

Foxinio commented Oct 30, 2025

Uh oh!

Foxinio commented Oct 30, 2025

Uh oh!

wojpok left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Foxinio left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wojpok commented Nov 3, 2025

Uh oh!

Foxinio commented Nov 4, 2025

Uh oh!

wojpok commented Nov 4, 2025

Uh oh!

wojpok left a comment

Choose a reason for hiding this comment

Uh oh!

Foxinio commented Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants