Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion exercises/forth/package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ tests:
source-dirs: test
dependencies:
- forth
- HUnit
- hspec
142 changes: 73 additions & 69 deletions exercises/forth/test/Tests.hs
Original file line number Diff line number Diff line change
@@ -1,74 +1,78 @@
{-# LANGUAGE OverloadedStrings #-}
import Test.HUnit (Assertion, (@=?), runTestTT, Test(..), Counts(..))
import System.Exit (ExitCode(..), exitWith)
import Forth (ForthError(..), evalText, empty, formatStack)
import Control.Monad (foldM)
import Data.Text (Text)

exitProperly :: IO Counts -> IO ()
exitProperly m = do
counts <- m
exitWith $ if failures counts /= 0 || errors counts /= 0 then ExitFailure 1 else ExitSuccess
import Control.Monad (foldM)
import Test.Hspec (Spec, describe, it, shouldBe)
import Test.Hspec.Runner (configFastFail, defaultConfig, hspecWith)

testCase :: String -> Assertion -> Test
testCase label assertion = TestLabel label (TestCase assertion)
import Forth (ForthError(..), empty, evalText, formatStack)

main :: IO ()
main = exitProperly $ runTestTT $ TestList
[ TestList forthTests ]

runTexts :: [Text] -> Either ForthError Text
runTexts xs = formatStack <$> foldM (flip evalText) empty xs

forthTests :: [Test]
forthTests =
[ testCase "no input, no stack" $
"" @=? formatStack empty
, testCase "numbers just get pushed onto the stack" $
Right "1 2 3 4 5" @=? runTexts ["1 2 3 4 5"]
, testCase "non-word characters are separators" $
-- Note the Ogham Space Mark ( ), this is a spacing character.
Right "1 2 3 4 5 6 7" @=? runTexts ["1\NUL2\SOH3\n4\r5 6\t7"]
, testCase "basic arithmetic" $ do
Right "-1" @=? runTexts ["1 2 + 4 -"]
Right "2" @=? runTexts ["2 4 * 3 /"]
, testCase "division by zero" $
Left DivisionByZero @=? runTexts ["4 2 2 - /"]
, testCase "dup" $ do
Right "1 1" @=? runTexts ["1 DUP"]
Right "1 2 2" @=? runTexts ["1 2 Dup"]
Left StackUnderflow @=? runTexts ["dup"]
, testCase "drop" $ do
Right "" @=? runTexts ["1 drop"]
Right "1" @=? runTexts ["1 2 drop"]
Left StackUnderflow @=? runTexts ["drop"]
, testCase "swap" $ do
Right "2 1" @=? runTexts ["1 2 swap"]
Right "1 3 2" @=? runTexts ["1 2 3 swap"]
Left StackUnderflow @=? runTexts ["1 swap"]
Left StackUnderflow @=? runTexts ["swap"]
, testCase "over" $ do
Right "1 2 1" @=? runTexts ["1 2 over"]
Right "1 2 3 2" @=? runTexts ["1 2 3 over"]
Left StackUnderflow @=? runTexts ["1 over"]
Left StackUnderflow @=? runTexts ["over"]
, testCase "defining a new word" $
Right "1 1 1" @=? runTexts [ ": dup-twice dup dup ;"
, "1 dup-twice"
]
, testCase "redefining an existing word" $
Right "1 1 1" @=? runTexts [ ": foo dup ;"
, ": foo dup dup ;"
, "1 foo"
]
, testCase "redefining an existing built-in word" $
Right "1 1" @=? runTexts [ ": swap dup ;"
, "1 swap"
]
, testCase "defining words with odd characters" $
Right "220371" @=? runTexts [": € 220371 ; €"]
, testCase "defining a number" $
Left InvalidWord @=? runTexts [": 1 2 ;"]
, testCase "calling a non-existing word" $
Left (UnknownWord "foo") @=? runTexts ["1 foo"]
]
main = hspecWith defaultConfig {configFastFail = True} specs

specs :: Spec
specs = describe "forth" $ do

-- As of 2016-10-02, there was no reference file
-- for the test cases in `exercism/x-common`.

let runTexts = fmap formatStack . foldM (flip evalText) empty

it "no input, no stack" $
formatStack empty `shouldBe` ""

it "numbers just get pushed onto the stack" $
runTexts ["1 2 3 4 5"] `shouldBe` Right "1 2 3 4 5"

it "non-word characters are separators" $
runTexts ["1\NUL2\SOH3\n4\r5 6\t7"] `shouldBe` Right "1 2 3 4 5 6 7"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to this PR, but now I would like to figure out the origin of this test...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


it "basic arithmetic" $ do
runTexts ["1 2 + 4 -"] `shouldBe` Right "-1"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is someone who does not read the README but only look at the tests, the lack of this comment might make things confusing. Do you think this is something to worry about? (Are there people who don't read the README?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that, ideally, people should get all the information needed to solve an exercise in the following order:

  • Running the tests - As a general rule this should be enough to know what went wrong.
  • Reading the hints - This could help when people get stuck.
  • Inspecting the test suite - Forcing people to read code should be a last resort solution, but I understand that some things are too complex to explain in English.

If there is someone who does not read the README but only look at the tests, the lack of this comment might make things confusing.

It could happen but, if HINTS.md is the right place to have the hints, we should enforce that there are no hints in the test suites or in the stub solutions.

IMHO, the only comments that should be in the test suites are the ones related to the test's implementation, to assist us in maintaining the tests suites.

An alternative would be to improve the tests and descriptions, if you think they are not good enough.

But this is just my opinion. If you think it is important to keep the comment there, I can update the PR immediately.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that, ideally, people should get all the information needed to solve an exercise in the following order:

This seems to make sense; I think it represents how we would interact with a library we are considering to use - we either run the tests or read its documentation (some people switch the order of these two don't they) and only as the resort will they read the code.

When I am solving an exercise I tend to look quite often at the tests (so I know what's coming up next and I know what's expected of me), but I can't speak for everyone.

I think my main question is - now that this line appears in the README, when someone looks at that, what do you think their reaction will be? Will it be "I'll immediately search for all instances of this   character to see what this is all about", or will it be "Why are you telling me this? I will forget about it". Well, it's not my place to predict how people will act.

An alternative would be to improve the tests and descriptions, if you think they are not good enough.

I'm not sure if "non-word characters are separators, including the Ogham Space Mark ( )" would be too much detail...

I question the case because visually the Ogham space mark is very easy to mistake for a minus sign, so someone looking at this might think this is five minus six, rather than five space six.

Perhaps I will take solace in the fact that most people probably don't fail this test because isSpace will do the right thing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think my main question is - now that this line appears in the README, when someone looks at that, what do you think their reaction will be? Will it be "I'll immediately search for all instances of this   character to see what this is all about", or will it be "Why are you telling me this? I will forget about it". Well, it's not my place to predict how people will act.

Maybe it doesn't belong in README...

After some thought, I think that we should simply remove the comment. It is too specific!

If it is part of testing non-word character, it doesn't deserve a comment more than NULL, SOH or tabs. If it is really a special case, it should have its own test.

What do you think?

Copy link
Copy Markdown
Member

@petertseng petertseng Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine the only reason it was there in the first place is so that people don't confuse it with the minus sign.

If I ever write the canonical data for this problem, I plan not to include the Ogham space. But that is a question for another day.

As far as I can tell, it is not a special case at all. Just another whitespace character. It's not really adding anything to the test.

So what does this mean! I noticed recently that I often imagine problems but don't have evidence that others will experience the same problem (in this case, the problem is "people might mistake the Ogham space for a minus sign"). So if you think the comment should simply be removed, let's do it. Otherwise we won't know if others have the same problem.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I ever write the canonical data for this problem, I plan not to include the Ogham space. But that is a question for another day.

I agree! It adds nothing!

So if you think the comment should simply be removed, let's do it. Otherwise we won't know if others have the same problem.

Done! 😄

runTexts ["2 4 * 3 /"] `shouldBe` Right "2"

it "division by zero" $
runTexts ["4 2 2 - /"] `shouldBe` Left DivisionByZero

it "dup" $ do
runTexts ["1 DUP" ] `shouldBe` Right "1 1"
runTexts ["1 2 Dup"] `shouldBe` Right "1 2 2"
runTexts ["dup" ] `shouldBe` Left StackUnderflow

it "drop" $ do
runTexts ["1 drop" ] `shouldBe` Right ""
runTexts ["1 2 drop"] `shouldBe` Right "1"
runTexts ["drop" ] `shouldBe` Left StackUnderflow

it "swap" $ do
runTexts ["1 2 swap" ] `shouldBe` Right "2 1"
runTexts ["1 2 3 swap"] `shouldBe` Right "1 3 2"
runTexts ["1 swap" ] `shouldBe` Left StackUnderflow
runTexts ["swap" ] `shouldBe` Left StackUnderflow

it "over" $ do
runTexts ["1 2 over" ] `shouldBe` Right "1 2 1"
runTexts ["1 2 3 over"] `shouldBe` Right "1 2 3 2"
runTexts ["1 over" ] `shouldBe` Left StackUnderflow
runTexts ["over" ] `shouldBe` Left StackUnderflow

it "defining a new word" $
runTexts [ ": dup-twice dup dup ;"
, "1 dup-twice" ] `shouldBe` Right "1 1 1"

it "redefining an existing word" $
runTexts [ ": foo dup ;"
, ": foo dup dup ;"
, "1 foo" ] `shouldBe` Right "1 1 1"

it "redefining an existing built-in word" $
runTexts [ ": swap dup ;"
, "1 swap" ] `shouldBe` Right "1 1"

it "defining words with odd characters" $
runTexts [": € 220371 ; €"] `shouldBe` Right "220371"

it "defining a number" $
runTexts [": 1 2 ;"] `shouldBe` Left InvalidWord

it "calling a non-existing word" $
runTexts ["1 foo"] `shouldBe` Left (UnknownWord "foo")