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

tests: fail-fast the test suites by default #190

Closed
rbasso opened this issue Jul 10, 2016 · 7 comments
Closed

tests: fail-fast the test suites by default #190

rbasso opened this issue Jul 10, 2016 · 7 comments

Comments

@rbasso
Copy link
Contributor

rbasso commented Jul 10, 2016

Some test suites are designed so that more complex cases come later, allowing users to incrementally solve the exercise, test by test.

I can think of three ways to deal with this usage case:

  • All tests enabled by default - This forces users to disable all the tests except the first before start solving the exercises. It's how it is now. 😩
  • All tests disable by default - This could save work for some users, but would be inconvenient for other users and would also break Travis-CI automated tests. ⛔
  • Fail-fast the tests - If the test suite fails displaying just the first error, everyone will be happy. ✅ 😄

Currently, all test suites use HUnit (trinary and octal use QuickCheck, but they will be deprecated by exercism/problem-specifications#279) because it was the only thing installed with the Haskell Platform. If we find a beautiful solution to fail-fast HUnit, we can use it immediately.

Alternatively, considering that we will migrate all exercises to Stack projects (#182, #189) and dependencies will not be a problem anymore, we can start to look at newer packages. I did some tests with tasty, tast-hunit and tasty-fail-fast, but they demand the user run the test with:

stack test --test-arguments --fail-fast

This is great, but it would be even better to have this as the default behavior. MichaelXavier/tasty-fail-fast#2

Does anyone knows a better solution?

@rbasso rbasso changed the title tests: fast-fail the test suites tests: fail-fast the test suites Jul 10, 2016
@rbasso
Copy link
Contributor Author

rbasso commented Jul 15, 2016

Finally got a complete solution with hspec instead of HUnit:

{-# OPTIONS_GHC -fno-warn-type-defaults #-}

import Data.Foldable     (for_)
import Test.Hspec        (describe, it, shouldBe)
import Test.Hspec.Runner (configFastFail, defaultConfig, hspecWith)

import LeapYear (isLeapYear)

main :: IO ()
main = hspecWith defaultConfig {configFastFail = True} spec
  where
    spec = describe "leap"
         $ describe "isLeapYear" $ for_ cases test

    test (label, year, expected) = it (label ++ " (" ++ show year ++ ")")
                                 $ isLeapYear year `shouldBe` expected

    cases = [ ("leap year"                  , 1996, False) -- Should be True!
            , ("standard and odd year"      , 1997, False)
            , ("standard even year"         , 1998, False)
            , ("standard nineteenth century", 1900, False)
            , ("standard eighteenth century", 1800, False)
            , ("leap twenty fourth century" , 2400, True )
            , ("leap y2k"                   , 2000, True ) ]

And it fails in the first test, without running the others: 😄

$ stack test
leap-0.0.0: test (suite: test)


leap
  isLeapYear
    leap year (1996) FAILED [1]

Failures:

  test/Tests.hs:25:
  1) leap.isLeapYear leap year (1996)
       expected: False
        but got: True

Randomized with seed 1218558111

Finished in 0.0004 seconds
1 example, 1 failure

Test suite failure for package leap-0.0.0
    test:  exited with: ExitFailure 1
Logs printed to console

Considering that hspec is not in the list of packages that we are using, it's probably safer to wait until we finish #185 before we start migrating the test suites. Until then, I would love to discuss alternative solutions.

@rbasso
Copy link
Contributor Author

rbasso commented Jul 17, 2016

So...anyone against to _fail-fast_ the tests by default?

@rbasso
Copy link
Contributor Author

rbasso commented Jul 18, 2016

@petertseng , what do you think about it?

@rbasso rbasso changed the title tests: fail-fast the test suites tests: fail-fast the test suites by default Jul 18, 2016
@petertseng
Copy link
Member

I'm not opposed, and it seems like almost every track I encounter does something similar (implementation depends on track)

@petertseng
Copy link
Member

Expanding on my earlier answer:

It seems makes sense to do the same in this track. Now I admit that I haven't been bothered too much by this, not enough that I get motivated to make the change myself. Nevertheless it is agreed that fast fail can make everyone happy. You will notice that #196 implies that at least some others wish to comment out tests and work iteratively! So I imagine the change would be received with gratitude by students in general.

@rbasso
Copy link
Contributor Author

rbasso commented Jul 20, 2016

Considering that migrating to hspec will require completely rewriting the tests, I think this may be a good opportunity to make sure that they are idiomatic and reflect what is available in x-common. I'm working on a standard Tests.hs file to use a template, to make the process at least a little easier.

@rbasso
Copy link
Contributor Author

rbasso commented Sep 3, 2016

Considering that the migration it's already decided and the migration is being tracked in #211, there is not reason to keep this issue open.

@rbasso rbasso closed this as completed Sep 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants