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 string-info wrapper for info messages #27

Merged
merged 5 commits into from
Jun 8, 2017
Merged

Add string-info wrapper for info messages #27

merged 5 commits into from
Jun 8, 2017

Conversation

jackfirth
Copy link
Sponsor Collaborator

Closes #26

With this change, it’s now possible for a check info value to print a message without quotes by wrapping the value string in a call to string-info. See the example added to the docs.

@jackfirth
Copy link
Sponsor Collaborator Author

Pinging @rmculpepper for review, on recommendation of @stamourv

Copy link
Contributor

@bennn bennn left a comment

Choose a reason for hiding this comment

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

Two thoughts about this:

1

Could you implement what you want outside of rackunit by:

  • making a value like (check-info (foo "hello"))
  • where foo is a struct that implements gen:custom-write
  • and displays hello in write-mode

How awkward would that be for your use-case?

2

IMO, I think it'd be better to expose a flag or parameter than add the string-info struct


A check-info structure stores information associated
with the context of execution of a check.}
A check-info structure stores information associated with the context of
Copy link
Contributor

Choose a reason for hiding this comment

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

should say "the context of the" ?

Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@jackfirth
Copy link
Sponsor Collaborator Author

jackfirth commented Jun 5, 2017

That could work fine for this use case, but I don't think it would work as a solution to #10 and I was hoping to add a very similar feature like (nested-info <check-info> ...) that displays as an indented list of infos. That would be much uglier to do with gen:custom-write, and I'd prefer a single solution that works for both of those use cases. I'm also considering a (repeated-info <value> ...) feature that makes (check-info 'foo (repeated-info 1 2 3) print like this:

---------------
FAILURE
foo:         1
foo:         2
foo:         3
---------------

I'm not yet sure if that's necessary for the extension library I'm making, but it definitely isn't possible to do with gen:custom-write.

@bennn
Copy link
Contributor

bennn commented Jun 5, 2017

Ok that makes more sense

(Does scribble have a text back-end we could re-use?)

@jackfirth
Copy link
Sponsor Collaborator Author

Do you mean some sort of Scribble subsystem for handling how Rackunit prints check info? I don't know much about Scribble's internals but it sounds unlikely.

@bennn
Copy link
Contributor

bennn commented Jun 6, 2017

I mean, a Scribble renderer that will convert nested structs into indented text and itemize structs into bullet-point text.

@jackfirth
Copy link
Sponsor Collaborator Author

There might be something like that in Scribble, but I wouldn't want to hold up this PR on figuring that out.

@jeapostrophe
Copy link
Contributor

I think this should be merged.

@jackfirth
Copy link
Sponsor Collaborator Author

Alas I don't have merge powers

@bennn bennn merged commit 748d165 into racket:master Jun 8, 2017
bennn pushed a commit that referenced this pull request Jun 19, 2017
This adds a private `location-info` struct alongside `string-info` from #27 to encapsulate rendering a source location with a relative directory path. This obsoletes the logic in both `rackunit/private/format` and `rackunit/text-ui` for handling a check info with key `'location` specially. This is not technically backwards compatible as it changes the observable behavior of `make-check-location`, but it does so in a way that's highly unlikely to affect existing users. See below for details.

* Remove unnecessary check-name? clause of run-tests

The `else` branch at the end of `check-name?` handles this already.

* Change check-info to use struct instead of define-struct

The latter is deprecated. Existing behavior is preserved with
#:constructor-name.

* Allow define-check-type to wrap info values

Needed for a future commit that adds a location-info struct.

* Add an internal location-info struct

This reproduces the behavior that `run-tests` and the default check
handler provide for the `location` info. It changes `check-location` to
automatically wrap the location value in the struct so `define-check`
doesn’t need to change. This breaks clients that construct a check info
with `check-location` and immediately extract the info value field
expecting it to be a `location/c`, but I don’t think anyone does that.

Future work could expose this to rack unit users, but it should
probably be done in terms of the `srcloc` structure instead.

* Bump version number

* Remove text-ui-util-test

This is now an internal implementation detail of
rackunit/private/check-info. Future work might test `location-info` and
`info-value->string` more thoroughly.

* Fix test-test w.r.t. location-info

Also trims some whitespace (auto formatted by Atom)

* Fix bug in prop:exn:srclocs

* Re-add tests for trim-current-directory in check-info tests

* Simplify default info-value printing logic
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.

None yet

3 participants