7
\$\begingroup\$

I am working through the UPenn CIS194 lectures and homework assignments in an effort to learn more about Haskell.

The first part of the first homework is focused on validating credit card numbers.

To validate each credit card number (passed in as an Integer), the Integer should be converted to a list of digits.

Beginning with the penultimate item in the list and moving toward the first item in the list, every other number is doubled.

The digits of all the numbers are summed.

If the resulting sum is divisible by 10, the credit card number is valid.

Three helper methods are recommended by the assignment on the way to the validate method.

  • Convert an Integer to a list of individual digits - toDigits
  • Double every other item in the list, starting with the penultimate item, moving toward the front. - doubleEveryOther
  • Sum all the resulting digits - sumDigits

I implemented some additional helpers.

  • reverse a list - myReverse [essentially an Integer-specific clone of reverse]
  • sum a list of integers - sumList [essentially a Integer-specific clone of sum]
  • fold helper that sums numbers less than 10 or calls sumDigits on numbers greater than 10 - sumDigitsFoldWorker

The functions in my solution are roughly equivalent to those in this prior question. I'm still digesting that post, but I think the main impact would be eliminating sumDigitsFoldWorker

I'd like feedback on the HUnit and QuickCheck tests I wrote while working the assignment.

module Homework01Test(
    testHomework01,
    ) where
import Homework01 (
    toDigits,
    toDigitsRev,
    myReverse,
    doubleEveryOther,
    sumDigits,
    sumList,
    sumDigitsFoldWorker,
    validate,
    checkSum,
    hanoi,
    )
import Test.QuickCheck(quickCheck)
import Test.HUnit (
    Test,
    Test(TestCase),
    Test(TestLabel),
    Test(TestList),
    assertEqual,
    runTestTT,
    )

--HUnit Tests

testToDigits1 :: Test
testToDigits1 = TestCase (assertEqual "for (toDigits (1234))," ([1, 2, 3, 4]) (toDigits 1234))

testToDigits2 :: Test
testToDigits2 = TestCase (assertEqual "for (toDigits (-17))," ([]) (toDigits (-17)))

testToDigitsRev1 :: Test
testToDigitsRev1 = TestCase (assertEqual "for (toDigitsRev (1234))," ([4, 3, 2, 1]) (toDigitsRev 1234))

testToDigitsRev2 :: Test
testToDigitsRev2 = TestCase (assertEqual "for (toDigitsRev (-17))," ([]) (toDigitsRev (-17)))

testDoubleEveryOther1 :: Test
testDoubleEveryOther1 = TestCase (assertEqual "for (doubleEveryOther [1, 2, 3])," ([1, 4, 3]) (doubleEveryOther [1, 2, 3]))

testDoubleEveryOther2 :: Test
testDoubleEveryOther2 = TestCase (assertEqual "for (doubleEveryOther [8, 7, 6, 5])," ([16, 7, 12, 5]) (doubleEveryOther [8, 7, 6, 5]))

testSumDigits1 :: Test
testSumDigits1 = TestCase (assertEqual "for (sumDigits [16, 7, 12 ,5])," 22 (sumDigits [16, 7, 12, 5]))

testSumDigits2 :: Test
testSumDigits2 = TestCase (assertEqual "for (sumDigits [])," (0) (sumDigits []))

testSumDigits3 :: Test
testSumDigits3 = TestCase (assertEqual "for (sumDigits [16, 1, 2, 3, 1, 1, 1])," (16) (sumDigits [16, 1, 2, 3, 1, 1, 1]))

testSumDigits4 :: Test
testSumDigits4 = TestCase (assertEqual "for (sumDigits [16, 1, 2, 3, 1, 1, 1])," (17) (sumDigits [16, 1, 2, 3, 1, 1, 1, 1]))

testValidate1 :: Test
testValidate1 = TestCase (assertEqual "for (validate 4012888888881881)," True (validate 4012888888881881))

testValidate2 :: Test
testValidate2 = TestCase (assertEqual "for (validate 4012888888881882)," False (validate 4012888888881882))

testCheckSum1 :: Test
testCheckSum1 = TestCase (assertEqual "for (checkSum 1386)," (18) (checkSum 1386))

testHanoi1 :: Test
testHanoi1 = TestCase (assertEqual "for (hanoi 2 \"a\" \"b\" \"c\")" ([("a","c"), ("a","b"), ("c","b")]) (hanoi 2 "a" "b" "c"))

testHanoi2 :: Test
testHanoi2 = TestCase (assertEqual "for (hanoi 3 \"left\" \"right\" \"middle\")" ([("left","right"),("left","middle"),("right","middle"),("left","right"),("middle","left"),("middle","right"),("left","right")]) (hanoi 3 "left" "right" "middle"))

hTests = TestList [TestLabel "Test toDigits pos"      testToDigits1,
                   TestLabel "Test toDigits neg"      testToDigits2,
                   TestLabel "Test toDigitsRev pos"   testToDigitsRev1,
                   TestLabel "Test toDigitsRev neg"   testToDigitsRev2,
                   TestLabel "Test doubleEveryOther1" testDoubleEveryOther1,
                   TestLabel "Test doubleEveryOther2" testDoubleEveryOther2,
                   TestLabel "Test sumDigits"         testSumDigits1,
                   TestLabel "Test sumDigits empty"   testSumDigits2,
                   TestLabel "Test sumDigits longer1" testSumDigits3,
                   TestLabel "Test sumDigits longer2" testSumDigits4,
                   TestLabel "Test checkSum"          testCheckSum1,
                   TestLabel "Test hanoi1"            testHanoi1,
                   TestLabel "Test hanoi2"            testHanoi2,
                   TestLabel "Test validate1"         testValidate1,
                   TestLabel "Test validate2"         testValidate2]
--QuickCheck Tests

prop_myReverse nums = myReverse nums == reverse nums
prop_sumList nums = sumList nums == sum nums

-- | Run tests for Homework01
testHomework01 :: IO ()
testHomework01 = do putStrLn "Homework01 Tests"
                    putStrLn "---HUnit---"
                    runTestTT hTests
                    putStrLn "---QuickCheck---"
                    quickCheck prop_myReverse
                    quickCheck prop_sumList

I am open to any feedback, but I have some specific questions.

  • Is there a more idiomatic, concise way to declare multiple tests for a single function?
  • How can I QuickCheck more of the code?
  • Is there a way to test methods without exporting them from a module? I have exported several internal helpers from Homework01 that I would rather keep private just for testing.
    • I am using stack Version 1.3.2
    • I can add the contents of my .cabal and stack.yaml if necessary
\$\endgroup\$
2
\$\begingroup\$

If using HUnit, I would define this kind of test more concisely by extracting some common functionality into a separate function and write them as follows:

mapAssertEquals :: (Show a, Show b, Eq b) => (a -> b) -> [(String, a, b)] -> Test
mapAssertEquals f testCases = TestList
  [ TestLabel label $ TestCase $ assertEqual (show input) expected $ f input
  | (label, input, expected) <- testCases
  ]

hTests :: Test
hTests = TestLabel "HUnit tests" $ TestList
  [ TestLabel "toDigits" $ mapAssertEquals toDigits
      [ ("pos", 1234, [1,2,3,4])
      , ("neg",  -17, [])
      ]
  , TestLabel "toDigitsRev" $ mapAssertEquals toDigitsRev
      [ ("pos", 1234, [4,3,2,1])
      , ("neg",  -17, [])
      ]
  , TestLabel "doubleEveryOther" $ mapAssertEquals doubleEveryOther
      [ ("odd length", [1,2,3], [1,4,3])
      , ("even length", [8,7,6,5], [16,7,12,5])
      ]
  , ...
  ]

The type signature involving typeclasses and the => symbol is described in the excellent Learn You A Haskell, and the $ function is also quite commonly used to reduce the number of nested parentheses.

I find the syntax for HSpec to be nicer than that for HUnit as it uses do-notation to define its tests rather than explicit lists. In HSpec the tests would look more like this:

spec :: Spec
spec = describe "HSpec tests" $ do

  describe "toDigits" $ do
    let input `shouldReturn` expected = toDigits input `shouldBe` expected
    it "works on positive numbers"          $ 1234  `shouldReturn` [1,2,3,4]
    it "returns an empty list on negatives" $ (-17) `shouldReturn` []

  describe "toDigitsRev" $ do
    let input `shouldReturn` expected = toDigitsRev input `shouldBe` expected
    it "works on positive numbers"          $ 1234  `shouldReturn` [4,3,2,1]
    it "returns an empty list on negatives" $ (-17) `shouldReturn` []

    it "is equivalent to (reverse . toDigits)" $ property $
      \n -> toDigitsRev n == reverse (toDigits (n::Integer))

  describe "doubleEveryOther" $ do
    let input `shouldReturn` expected = doubleEveryOther input `shouldBe` expected
    it "works on odd-length lists"  $ [1,2,3]   `shouldReturn` [1,4,3]
    it "works on even-length lists" $ [8,7,6,5] `shouldReturn` [16,7,12,5]

  ...

Note that this also makes it straightforward to include property-based QuickCheck tests via the property function as shown.

The trick to running QuickCheck on more of your code is identifying more simple algebraic properties that relate the various functions. One example, \n -> toDigitsRev n == reverse (toDigits n), is shown above. Other ones that I can think of here are:

\ns -> even (length ns) ==>
  doubleEveryOther (reverse (doubleEveryOther ns)) == reverse (map (*2) ns)

\ns -> sumDigits ns == sum (concatMap toDigits ns) -- this one fails, maybe a bug?

For the validate function I can't think of any very simple properties, although you might want to say things like if the input is valid then the same input with one mistyped digit is invalid, as is the same input with two digits transposed.

There is no way to test methods without exporting them, unless you put the test code within the module itself. Test code is just code so the same accessibility rules apply as for everything else. That said, it's often considered bad style to directly test internal code, as it could be seen to be an overspecification of the implementation which constrains the kinds of changes you can make in the future.

Personally, in this sort of case I think I'd create a Module.Internal module containing the implementation, and have the main Module just expose the public interface via re-exports but have the tests work against Module.Internal instead.

Hope that helps!

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for the reply! I wasn't familiar with the $ and => operators, so I added some explanation. $ changes the precedence of function application (reduces the number of parentheses required). $ operator => is used in type signatures to specify class constraints. => operator \$\endgroup\$ – jeyoor Jul 10 '17 at 12:31
  • 1
    \$\begingroup\$ Yep, well done, those are good references. I have added your links to my answer, in case they're of use to others. \$\endgroup\$ – Dave Turner Jul 10 '17 at 16:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.