From 972454f0cc029f395179ef582911be0e2bbd84c9 Mon Sep 17 00:00:00 2001 From: Michael Snoyman Date: Sun, 12 Nov 2017 17:27:06 +0200 Subject: [PATCH] Future proofing test suites --- posts.yaml | 4 ++ posts/future-proofing-test-suites.md | 64 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 posts/future-proofing-test-suites.md diff --git a/posts.yaml b/posts.yaml index 1756b48..e2bb754 100644 --- a/posts.yaml +++ b/posts.yaml @@ -1,3 +1,7 @@ +- file: posts/future-proofing-test-suites.md + title: Future proofing test suites + time: 2017-11-12T17:00:00Z + description: "A small piece of advice about including test suites that may fail over time" - file: posts/effective-ways-help-from-maintainers.md title: Effective Ways to Get Help from Maintainers time: 2017-10-23T08:45:00Z diff --git a/posts/future-proofing-test-suites.md b/posts/future-proofing-test-suites.md new file mode 100644 index 0000000..58f5a3c --- /dev/null +++ b/posts/future-proofing-test-suites.md @@ -0,0 +1,64 @@ +I'll start with the specific case I've seen pop up a few times +recently, and then expand to the general. If you're a package author +who has been affected by this, please note: I'm putting this +information into a blog post since it's easier to state this once and +link to it rather than rewrite an explanation on lots of different bug +trackers. + +[hlint](https://www.stackage.org/package/hlint) is a great tool for +getting advice on improving your Haskell codebase (another great Neil +Mitchell product). And as such tools go, hlint has new versions which +improve its ability to provide useful advice. This means that, +sometimes, code which triggered no hlint warnings previously may +suddenly present with such warnings under a new hlint version. + +Twice recently in my Stackage curation, I've seen a number of test +suites fail, even though the code for those packages was +unmodified. It turns out that the upgrade to a new version of hlint +caused a previously successful test suite to now fail. Clearly the +code isn't suddenly broken because a new version of hlint has been +released, but as far as the diagnostics of test suite failures are +concerned, that's exactly what happened. + +## Recommendation + +I do strongly recommend projects use hlint to get code +improvements. And I've seen some great results with using it as part +of the CI process, such as on Stack. (For the record: it wasn't my +idea and I didn't implement it. I was just pleasantly surprised when +my PRs failed because I had some style errors.) However, making the +test suite for the entire package fail because of a new version of +hlint is too much. Therefore: + +* __DO__ Have some way to run hlint from your CI process, if you + want these warnings to block PRs. There are two approaches I can + think of: + + * The way Stack does it: have a + [separate part of the build matrix](https://github.com/commercialhaskell/stack/blob/46121be1b96465f1164e3f84cafa19c7369da9cc/.travis.yml#L39) + just for style errors. The cabal file for the project itself + knows nothing about hlint. + * Via a test suite in your cabal file which is disabled by + default. Then: turn on that test suite with a flag from your CI + configuration. + +* __DON'T__ Set up your package which is uploaded to Hackage/built + by Stackage such that it will fail if a style-based error occurs. + +## General recommendation + +The general takeaway from this is: when you're building your code on +CI, be as strict as you want. Set high standards, block PRs, call +master broken, for whatever trivial or non-trivial issues you deem +worthy. Turn on `-Wall -Werror`, respect hlint, error out if someone +uses tabs\* or includes trailing whitespace. That's all good. + +\* Cue necessary tabs-vs-spaces argument + +_However_, when you're releasing your code elsewhere, make the tests +as lenient as possible on optional features. If the code fails to +build: that's a problem. If the code builds, but returns incorrect +runtime results: that's a problem. These should stop build systems +like Stackage from including your package. But stylistic issues, or +newly introduced warnings from the compiler, or myriad other issues, +should not trigger a failure for downstream consumers of your package.