Hacker News Reader

Comment by mtlynch | Hacker News Reader

>The "Parse, don't validate" paper is classic IMHO.

Yeah, I find it so baffling that 90% of programmers I talk to have never heard of it. My circle is more Go/Python/C++ folks, so maybe it's more well-known in functional programming circles.

>I disagree with "Don't put logic in tests", with the example provided being a problem with using strings where a URI type is needed instead. Perhaps the source of my disagreement is I hold that test code is production code due to test suite failure(s) during an automated build stops desired deployment.

Yeah, I think that's a fair criticism. I think the specifics of the example could be better, but I think the important underlying message is that even something that seems simple like a string concatenation is added complexity that can mask a bug in a test.

View on HN

>> I disagree with "Don't put logic in tests", with the example provided being a problem with using strings where a URI type is needed instead. Perhaps the source of my disagreement is I hold that test code is production code due to test suite failure(s) during an automated build stops desired deployment.

> Yeah, I think that's a fair criticism. I think the specifics of the example could be better, but I think the important underlying message is that even something that seems simple like a string concatenation is added complexity that can mask a bug in a test.

I didn't mean to criticize so much as identify why I consider test code to be the same as production code.

Continuing with the example provided, the string concatenation is not the problem this test identifies IMHO. Instead, it is that:

  nav.getCurrentUrl()

Returns a `String` instead of a type which disallows the formulation of the problematic `assertEquals` to begin with.

In a more general sense, I have found treating test suites the same as one would production code (refactoring, commenting, sometimes testing support logic used to define tests, etc.) has led to tests benefiting the same way. This approach also has had a twofold benefit of "keeping the same energy" when producing all source artifacts along with serving as a great way to onboard new team members.

All the usual caveats apply, of course. YMMV, IMHO, etc. :-)

> Returns a `String` instead of a type which disallows the formulation of the problematic `assertEquals` to begin with.

I'm not sure what the best attribution would be but "Make illegal states unrepresentable" would be a fantastic addition to this list pairing well with "parse, don't validate".

A stricter type would force you to parse the URL and would either fix the error (because cleaning trailing/leading slashes might make sense here) or throw a clear error from the parser.

It can be slightly more verbose when you just want to write a string in your test for convenience but can (and does) save a lot of debugging pain for less trivial cases.

>> Returns a `String` instead of a type which disallows the formulation of the problematic `assertEquals` to begin with.

> I'm not sure what the best attribution would be but "Make illegal states unrepresentable" would be a fantastic addition to this list pairing well with "parse, don't validate".

The phrases I have seen describing using types to make illegal states incapable of being represented are "programming with types"[0] and "type level programming"[1].

HTH

0 - https://www.manning.com/books/programming-with-types

1 - https://rebeccaskinner.net/posts/2021-08-25-introduction-to-...

What you're talking about is changing the interface of the code under test. You usually can't do that. If the interface outputs a string then you need to deal with a string.

Ideally a test should be an extremely literal interpretation of the spec. The test in the example I would read as "the output should be the result of a string concatenation between the base URL string and this string". The problem is that spec doesn't exist because it's stupid and wrong. This test suite implements a bug-ridden, ad hoc URL builder.

The real spec is probably "the output should be a string containing a valid URL for the photos page". I see two options for writing this test:

1. Compare to a "trusted" string literal which conforms to the spec (ie. to be a valid URL for the photos page),

2. Decode the string using a "trusted" URL decoder and validate it at the URL level.

I think they are equally valid options and a matter of taste and convenience but both accurately reflect the spec, unlike the example.

The second option is essentially playing off two bits of code against each other, like testing a forwards operation against a reverse operation. This works if both bits of code are completely independent and therefore extremely unlikely to accidentally compensate for each other's bugs. If the URL decoder is in a widely used standard library then this could be considered OK (the robustness principle explicitly makes things asymmetric, though, so a URL decoder with a "strict" mode would be most appropriate). But ad hoc code written in a test suite is definitely not OK!

Reply on news.ycombinator.com