Michael2109 / cobalt

The Cobalt programming language
GNU Lesser General Public License v3.0
37 stars 10 forks source link

Refactor tests #460

Closed FilipJanitor closed 6 years ago

FilipJanitor commented 6 years ago

Some tests contain a lot of duplicated code.

let codeAdd = "x + 100"
let testAdd = TestCase $ assertEqual codeAdd
                   (ABinary Add (Var "x") (IntConst 100))
                   (case (parse aExpr "" codeAdd) of
                       Left  e -> error $ show e
                       Right x -> x)

let codeSubtract = "x - 100"
let testSubtract = TestCase $ assertEqual codeSubtract
                   (ABinary Subtract (Var "x") (IntConst 100))
                   (case (parse aExpr "" codeSubtract) of
                       Left  e -> error $ show e
                       Right x -> x)

...

This might be possibly rewritten to something like

test code result = TestCase $ assertEqual code
    result
    (case (parse aExpr "" code) of
        Left  e -> error $ show e
        Right x -> x)

--then in the testExprParser function 
let codeAdd = "x + 100"
let resultAdd = (ABinary Add (Var "x") (IntConst 100))
let codeSubtract = "x - 100"
let resultSubstract = (ABinary Subtract (Var "x") (IntConst 100))

-- instead of calling testAdd we can call (test codeAdd resultAdd)
Michael2109 commented 6 years ago

This makes sense. We could have this in a test utils function. Also we should also add one for checking the left side too for if there are parsing errors.

FilipJanitor commented 6 years ago

That is the question we mentioned in #431 too - what should actually be tested when we expect failure. Simply check if failure occured? Do we want to check for specific failure? Because if not, we can use the hack I used in CommandLineUtilsTest - simply wrap the result in some structure and by this we can reuse the test function even for failing tests.

Michael2109 commented 6 years ago

I think all errors we check should ensure the error occurs on a particular line/column at least using ParseError. This will help us a lot as otherwise we may miss big issues where error messages aren't being displayed when they should be. We need to try being as precise as possible.

Michael2109 commented 6 years ago

There's one problem with this. The testParseSuccess function I've added is now in a parser test utils module. If an error occurs with a parser it outputs the code that was being parsed and the error message.
We need to know where the error actually occurred. Currently it just says it occurred in the module where testParseSuccess exists. We need it to instead specify the testing module instead. E.g

100.50988678f - TrivialError (SourcePos {sourceName = "", sourceLine = Pos 1, sourceColumn = Pos 1} :| []) (Just (Tokens ('1' :| ""))) (fromList [Tokens ('f' :| "")])
CallStack (from HasCallStack):
  error, called at test/tests\TestUtil\ParserTestUtil.hs:12:20 in main:TestUtil.ParserTestUtil

This is good in that it gives us the error but without looking it up we don't know which specific test caused this. I wonder if we can get the whole stack trace.