-
Notifications
You must be signed in to change notification settings - Fork 81
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 optional location information on test assertion functions #291
Conversation
This location information is used to give more helpful error messages if the assertions fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Co-authored-by: ngoguey <ngoguey@student.42.fr>
Thanks @Ngoguey42. I'd appreciate a second review / opinion on the API from others who might use the feature, since this is a change to a fairly core set of functions. @gs0510, @icristescu: any thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so useful! I can already seeing it being very advantageous for ofronds
:)
[FAIL] fail 1 pos. | ||
|
||
┌──────────────────────────────────────────────────────────────────────────────┐ | ||
│ [FAIL] check 0 here. │ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the error messages are descriptive (which is great!) but the 1 pos.
confuses me a bit. What are the 1 pos.
or 0 here
supposed to indicate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that looks confusing here; it's actually a general feature of Alcotest output: tests are printed with their index + name, so for a more normal set of tests it might look like this:
[OK] FS 0 Basic operations on contents.
[OK] FS 1 Basic operations on nodes.
[OK] FS 2 Basic operations on commits.
[OK] FS 3 Basic operations on branches.
[OK] FS 4 Hash operations on trees.
[OK] FS 5 Basic merge operations.
[OK] FS 6 Basic operations on slices.
[OK] FS 7 Test merges on tree updates.
[OK] FS 8 Tree caches and hashconsing.
[OK] FS 9 Complex histories.
[OK] FS 10 Empty stores.
[OK] FS 11 Private node manipulation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh okay, that makes sense. Thanks! LGTM! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks good to me, thanks!
…lwt (1.4.0) CHANGES: - Add `?here` and `?pos` arguments to the test assertion functions. These can be used to pass information about the location of the call-site, which is displayed in failing test output. (mirage/alcotest#291, @craigfe)
…lwt (1.4.0) CHANGES: - Add `?here` and `?pos` arguments to the test assertion functions. These can be used to pass information about the location of the call-site, which is displayed in failing test output. (mirage/alcotest#291, @craigfe) - Add a pretty-printer for the exception raised by `Alcotest.check` and related functions. This allows them to be used outside of an Alcotest test runner for making general assertions. (mirage/alcotest#296, @craigfe) - Add `--bail` option (and corresponding `ALCOTEST_BAIL` environment variable), which causes Alcotest to terminate after the first test failure. (mirage/alcotest#298, @craigfe)
…lwt (1.4.0) CHANGES: - Add `?here` and `?pos` arguments to the test assertion functions. These can be used to pass information about the location of the call-site, which is displayed in failing test output. (mirage/alcotest#291, @craigfe) - Add a pretty-printer for the exception raised by `Alcotest.check` and related functions. This allows them to be used outside of an Alcotest test runner for making general assertions. (mirage/alcotest#296, @craigfe) - Add `--bail` option (and corresponding `ALCOTEST_BAIL` environment variable), which causes Alcotest to terminate after the first test failure. (mirage/alcotest#298, @craigfe)
This location information is used to give more helpful error messages if the assertions fail.
CC: @mefyl.