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

Quote and escape Observable strings and chars #296

Closed
wants to merge 6 commits into from

Conversation

jmid
Copy link
Collaborator

@jmid jmid commented Dec 6, 2024

When recently using QCheck's function generation support, I noticed an issue with its printing of chars and strings.
Consider these two examples:

open QCheck

let test_char =
  Test.make
    (triple char char (fun1 Observable.char bool))
    (fun (c1,c2,f) -> Fn.apply f c1 = Fn.apply f c2)

let _ = QCheck_base_runner.run_tests ~verbose:true [test_char]

let test_string =
  Test.make
    (triple string string (fun1 Observable.string bool))
    (fun (s1,s2,f) -> Fn.apply f s1 = Fn.apply f s2)

let _ = QCheck_base_runner.run_tests ~verbose:true [test_string]

resulting in output such as:

generated error fail pass / total     time test name
[✗]    1    0    1    0 /  100     0.0s anon_test_1

--- Failure --------------------------------------------------------------------

Test anon_test_1 failed (15 shrink steps):

('a', '\142', {� -> true; _ -> false})
================================================================================
failure (1 tests failed, 0 tests errored, ran 1 tests)
generated error fail pass / total     time test name
[✗]    2    0    1    1 /  100     0.0s anon_test_2

--- Failure --------------------------------------------------------------------

Test anon_test_2 failed (27 shrink steps):

("a", "", { -> false; _ -> true})

Note the not-so-helpful function domain printing: � -> true and -> false.

This PR therefore offers a quickfix to these two printers, aligning them with the arbitrary printers, and thus offering a uniform print out:

generated error fail pass / total     time test name
[✗]    1    0    1    0 /  100     0.0s anon_test_1

--- Failure --------------------------------------------------------------------

Test anon_test_1 failed (18 shrink steps):

('a', '_', {'_' -> false; _ -> true})
================================================================================
failure (1 tests failed, 0 tests errored, ran 1 tests)
generated error fail pass / total     time test name
[✗]    1    0    1    0 /  100     0.0s anon_test_2

--- Failure --------------------------------------------------------------------

Test anon_test_2 failed (34 shrink steps):

("a", "", {"" -> false; _ -> true})
================================================================================
failure (1 tests failed, 0 tests errored, ran 1 tests)

Looking further,

  • I was annoyed by having to duplicate the ad-hoc printers from arbitrary one more time, rather than use the Print module, supposed to be used for this
  • I also noticed that QCheck2.Print.char and QCheck2.Print.string already behave like the above, creating a QCheck.Print / QCheck2.Print mismatch for chars and strings 😱

I'll therefore file a sister-PR with a breaking change to QCheck.Print.{char,string} restoring uniformity and order to the universe... ⭐ ...then we discuss the preferred solution and whether anyone's (expect) tests may be broken by it.

@jmid
Copy link
Collaborator Author

jmid commented Dec 7, 2024

That the Print.{char,string} combinators do not properly quote and escape is a genuine problem

Since #297 does not break any reverse dependencies on the opam repo, I plan to close this one and go with #297 unless I receive any objections.

@jmid
Copy link
Collaborator Author

jmid commented Dec 12, 2024

Closing this in favour of #297

@jmid jmid closed this Dec 12, 2024
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