tdammers / ginger

A Haskell implementation of the Jinja template language.
MIT License
77 stars 13 forks source link

Test failure with http-types 0.11 #13

Closed DanBurton closed 6 years ago

DanBurton commented 6 years ago

As seen on the Stackage build server:

ginger: Process exited with ExitFailure 1: dist/build/tests/tests (pending: 146$
, failures: 0)

      "urlencode":
  FAIL
        test/Text/Ginger/SimulationTests.hs:1294:
        expected: "a%2Fb%20c"
         but got: "a%2fb%20c"

1 out of 249 tests failed (0.22s)

What is your take on the severity of this? I'm not necessarily recommending that any action be taken on your part, since I think that http-types should return to uppercasing.

See also: https://github.com/fpco/stackage/issues/3226

tdammers commented 6 years ago

I don't think this requires any more action than making the test case more lenient (i.e., case insensitive in the percent encoded items).

After all, the urlencode Ginger function promises nothing more than the underlying Haskell function it wraps: producing a string that is a correctly urlencoded representation of the input. Both upper- and lowercase hex codes are OK as far as I am concerned, I have no strong preference either way (although I do think changing this behavior without at least a "minor" version bump isn't exactly a nice move).

tdammers commented 6 years ago

Actually, making the test more lenient in just the right way is harder than you'd think, since you want it to be case-insensitive only on the percent-encoded parts, not the rest. Hmm.

DanBurton commented 6 years ago

I've raised an issue on http-types and the maintainer has agreed to release a new version that restores the uppercase behavior. https://github.com/aristidb/http-types/issues/83

We will be skipping http-types 0.11 on stackage, so you don't need to take any further action as far as stackage is concerned.

tdammers commented 6 years ago

Alright, thanks for the heads up though!

aristidb commented 6 years ago

http-types 0.12 (released today) should fix the issue

tdammers commented 6 years ago

029547 changes dependency bounds such that http-types 0.11 will never be used.