Skip to content
This repository has been archived by the owner on Oct 9, 2019. It is now read-only.

FR: Add expect functions which take custom formatters #224

Closed
rhofour opened this issue Dec 24, 2017 · 4 comments
Closed

FR: Add expect functions which take custom formatters #224

rhofour opened this issue Dec 24, 2017 · 4 comments

Comments

@rhofour
Copy link

rhofour commented Dec 24, 2017

Consider a test like this:

binaryString1 =
    String.fromList [ 'a', '\x00', 'b', '\x01', 'c' ]


binaryString2 =
    String.fromList [ 'a', '\x00', 'b', 'c' ]


suite : Test
suite =
    test "Check if these strings are equal" <|
        (\_ ->
            Expect.equal
                binaryString1
                binaryString2
        )

When run the output looks like this:

✗ Check if these strings are equal

"a\0bc"                                          
╷                                                
│ Expect.equal                                   
╵                                                
"a\0bc"

Notice that it's not possible to tell what's wrong because of how these strings are formatted. I've personally lost quite some time debugging something very similar to this.

I'm proposing adding a couple of new functions like Expect.equalFormatted:
Expect.equalFormatted : (a -> String) -> a -> a -> Expectation

It could be used like so:

binaryString1 =
    String.fromList [ 'a', '\x00', 'b', '\x01', 'c' ]


binaryString2 =
    String.fromList [ 'a', '\x00', 'b', 'c' ]


toHexStr : Int -> String
toHexStr x =
    let
        hexChars =
            [ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' ]

        first =
            Maybe.withDefault '?' (List.head (List.drop (x // 16) hexChars))

        second =
            Maybe.withDefault '?' (List.head (List.drop (rem x 16) hexChars))
    in
        String.cons first (String.fromChar second)


toBytes : String -> String
toBytes s =
    let
        codes =
            List.map Char.toCode (String.toList s)

        formattedCodes =
            List.map (\x -> "'\\x" ++ (toHexStr x) ++ "'") codes
    in
        "(String.fromList [" ++ (String.join ", " formattedCodes) ++ "])"


formattedSuite : Test
formattedSuite =
    test "Check if these strings are equal with custom formatting" <|
        (\_ ->
            Expect.equalFormatted
                toBytes
                binaryString1
                binaryString2
        )

This gives much more useful output with the only cost being expanding the API:

✗ Check if these strings are equal with custom formatting

(String.fromList ['\x61', '\x00', '\x62', '\x63'])
╷
│ Expect.equal
╵
(String.fromList ['\x61', '\x00', '\x62', '\x01', '\x63'])

I have the code written for this. I'll send a PR soon, but I'm open to doing things differently. I just want to see some way of adding custom formatters.

@rtfeldman
Copy link
Member

rtfeldman commented Dec 24, 2017

I just want to see some way of adding custom formatters.

@rhofour sorry I seem to have miscommunicated what I meant, so let me be clearer about this: I do not want to see some way of adding custom formatters. Please do not make a PR for adding them.

I'm open to considering custom formatters as a last resort, but I strongly believe that tests should be as low-friction to write as possibles. Custom formatters are not something I want to impose on test authors until after we've tried and failed to solve the specific problem outlined in #217 in a way that Just Works, without anyone having to use elm-test differently that they do today.

Again, sorry I didn't communicate that clearly the first time.

@rtfeldman
Copy link
Member

I opened #225 to describe what I had in mind instead!

@rhofour
Copy link
Author

rhofour commented Dec 24, 2017

I'm not sure you're completely following what I'm suggesting. This would involve the addition of new functionality, but it's perfectly backwards-compatible. That is, everything would continue to work as it always has and there would be no additional friction. People could use custom formatters if they wanted, but could also completely ignore them and use elm-test like they do today.

I'll take a look at #225. I certainly think a solution that Just Works would be great, but I think it would be much more difficult to actually implement (and there could still be other uses for custom formatters).

Edit: On looking closer I don't think implementing #225 would be as difficult as I imagined. However, I still have a use for custom formatters beyonds #217. Should I describe my usecase here or open a new issue just for it?

@rtfeldman
Copy link
Member

I'm not sure you're completely following what I'm suggesting. This would involve the addition of new functionality [...] People could use custom formatters if they wanted

The bolded part is the full extent of what I object to. 😄

Making the API bigger means there's more to learn and more choices for people to make in the course of using the library. Both of these are undesirable, all else being equal.

Since expanding the API (of any library) is by default a negative, I want to avoid it whenever there appears to be a fine solution that doesn't require expanding the API. 🙂

I still have a use for custom formatters beyonds #217. Should I describe my usecase here or open a new issue just for it?

I'd say open a new issue, but I would describe the symptoms in that issue—e.g. "I want to do _____ because ______ but I currently have no way to do that" not in terms of a feature request for one possible solution to those symptoms (e.g. "I want custom formatters because _______").

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants