Make expected errors order-agnostic

Fixes #12
This commit is contained in:
Jeroen Engels 2020-05-17 11:47:52 +02:00
parent ee8f294c18
commit c76d4d972f
3 changed files with 426 additions and 14 deletions

View File

@ -145,12 +145,15 @@ type alias CodeInspector =
{-| An expectation for an error. Use [`error`](#error) to create one. {-| An expectation for an error. Use [`error`](#error) to create one.
-} -}
type ExpectedError type ExpectedError
= ExpectedError = ExpectedError ExpectedErrorDetails
{ message : String
, details : List String
, under : Under type alias ExpectedErrorDetails =
, fixedSource : Maybe String { message : String
} , details : List String
, under : Under
, fixedSource : Maybe String
}
type Under type Under
@ -946,9 +949,120 @@ checkIfLocationIsAmbiguousInSourceCode sourceCode error_ under =
-- RUNNING THE CHECKS -- RUNNING THE CHECKS
type alias ReorderState =
{ expectedErrors : List ExpectedError
, reviewErrors : List ReviewError
, pairs : List ( ExpectedError, ReviewError )
, expectedErrorsWithNoMatch : List ExpectedError
}
reorderErrors : CodeInspector -> ReorderState -> ( List ExpectedError, List ReviewError )
reorderErrors codeInspector reorderState =
case reorderState.expectedErrors of
[] ->
( List.reverse <| reorderState.expectedErrorsWithNoMatch ++ List.map Tuple.first reorderState.pairs
, List.reverse <| reorderState.reviewErrors ++ List.map Tuple.second reorderState.pairs
)
((ExpectedError expectedErrorDetails) as expectedError) :: restOfExpectedErrors ->
case findBestMatchingReviewError codeInspector expectedErrorDetails reorderState.reviewErrors { error = Nothing, confidenceLevel = 0 } of
Just reviewError ->
reorderErrors codeInspector
{ reorderState
| pairs = ( expectedError, reviewError ) :: reorderState.pairs
, reviewErrors = removeFirstOccurrence reviewError reorderState.reviewErrors
, expectedErrors = restOfExpectedErrors
}
Nothing ->
reorderErrors codeInspector
{ reorderState
| expectedErrorsWithNoMatch = expectedError :: reorderState.expectedErrorsWithNoMatch
, expectedErrors = restOfExpectedErrors
}
removeFirstOccurrence : a -> List a -> List a
removeFirstOccurrence elementToRemove list =
case list of
[] ->
[]
x :: xs ->
if x == elementToRemove then
xs
else
x :: removeFirstOccurrence elementToRemove xs
findBestMatchingReviewError : CodeInspector -> ExpectedErrorDetails -> List ReviewError -> { error : Maybe ReviewError, confidenceLevel : Int } -> Maybe ReviewError
findBestMatchingReviewError codeInspector expectedErrorDetails reviewErrors bestMatch =
case reviewErrors of
[] ->
bestMatch.error
reviewError :: restOfReviewErrors ->
let
confidenceLevel : Int
confidenceLevel =
matchingConfidenceLevel codeInspector expectedErrorDetails reviewError
in
if confidenceLevel > bestMatch.confidenceLevel then
findBestMatchingReviewError
codeInspector
expectedErrorDetails
restOfReviewErrors
{ error = Just reviewError, confidenceLevel = confidenceLevel }
else
findBestMatchingReviewError
codeInspector
expectedErrorDetails
restOfReviewErrors
bestMatch
matchingConfidenceLevel : CodeInspector -> ExpectedErrorDetails -> ReviewError -> Int
matchingConfidenceLevel codeInspector expectedErrorDetails reviewError =
if expectedErrorDetails.message /= Rule.errorMessage reviewError then
0
else
case expectedErrorDetails.under of
Under under ->
if codeInspector.getCodeAtLocation (Rule.errorRange reviewError) /= Just under then
1
else
2
UnderExactly under range ->
if codeInspector.getCodeAtLocation (Rule.errorRange reviewError) /= Just under then
1
else if range /= Rule.errorRange reviewError then
2
else
3
checkAllErrorsMatch : SuccessfulRunResult -> List ExpectedError -> Expectation checkAllErrorsMatch : SuccessfulRunResult -> List ExpectedError -> Expectation
checkAllErrorsMatch runResult expectedErrors = checkAllErrorsMatch runResult unorderedExpectedErrors =
checkErrorsMatch runResult expectedErrors runResult.errors let
( expectedErrors, reviewErrors ) =
reorderErrors
runResult.inspector
{ expectedErrors = unorderedExpectedErrors
, reviewErrors = runResult.errors
, pairs = []
, expectedErrorsWithNoMatch = []
}
in
--checkErrorsMatch runResult unorderedExpectedErrors runResult.errors
checkErrorsMatch runResult expectedErrors reviewErrors
|> List.reverse |> List.reverse
|> (\expectations -> Expect.all expectations ()) |> (\expectations -> Expect.all expectations ())
@ -972,6 +1086,7 @@ checkErrorsMatch runResult expectedErrors errors =
checkErrorMatch : CodeInspector -> ExpectedError -> ReviewError -> (() -> Expectation) checkErrorMatch : CodeInspector -> ExpectedError -> ReviewError -> (() -> Expectation)
checkErrorMatch codeInspector ((ExpectedError expectedError_) as expectedError) error_ = checkErrorMatch codeInspector ((ExpectedError expectedError_) as expectedError) error_ =
-- TODO Look here for comparison
Expect.all Expect.all
[ \() -> [ \() ->
(expectedError_.message == Rule.errorMessage error_) (expectedError_.message == Rule.errorMessage error_)
@ -1007,7 +1122,8 @@ checkMessageAppearsUnder codeInspector error_ (ExpectedError expectedError) =
, \() -> , \() ->
(codeAtLocation == under) (codeAtLocation == under)
|> Expect.true (FailureMessage.underMismatch error_ { under = under, codeAtLocation = codeAtLocation }) |> Expect.true (FailureMessage.underMismatch error_ { under = under, codeAtLocation = codeAtLocation })
, \() -> codeInspector.checkIfLocationIsAmbiguous error_ under , \() ->
codeInspector.checkIfLocationIsAmbiguous error_ under
] ]
UnderExactly under range -> UnderExactly under range ->

View File

@ -142,15 +142,15 @@ type Foo = Bar | Baz"""
|> Review.Test.runWithProjectData project (rule []) |> Review.Test.runWithProjectData project (rule [])
|> Review.Test.expectErrors |> Review.Test.expectErrors
[ Review.Test.error [ Review.Test.error
{ message = "Type constructor `Bar` is not used."
, details = details
, under = "Bar"
}
, Review.Test.error
{ message = "Type constructor `Baz` is not used." { message = "Type constructor `Baz` is not used."
, details = details , details = details
, under = "Baz" , under = "Baz"
} }
, Review.Test.error
{ message = "Type constructor `Bar` is not used."
, details = details
, under = "Bar"
}
] ]
] ]

View File

@ -0,0 +1,296 @@
module Review.Test.ExpectedErrorOrderTest exposing (all)
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.Node as Node exposing (Node)
import Review.Rule as Rule exposing (Error, Rule)
import Review.Test
import Test exposing (Test, describe, test)
all : Test
all =
describe "Review.Test - order of tests"
[ differentMessageTest
, sameMessageTest
]
differentMessageTest : Test
differentMessageTest =
describe "when message is different for every error"
[ test "should match a single error to the single expected error" <|
\() ->
"""module MyModule exposing (b)
a = foo"""
|> Review.Test.run ruleWithDifferentMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
]
, test "should match multiple errors with the expected errors that are already in the correct order" <|
\() ->
"""module MyModule exposing (b)
a = foo
bar
baz
"""
|> Review.Test.run ruleWithDifferentMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
, Review.Test.error
{ message = "Detected bar"
, details = details
, under = "bar"
}
, Review.Test.error
{ message = "Detected baz"
, details = details
, under = "baz"
}
]
, test "should match multiple errors with the expected errors that are in reverse order" <|
\() ->
"""module MyModule exposing (b)
a = foo
bar
baz
"""
|> Review.Test.run ruleWithDifferentMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Detected baz"
, details = details
, under = "baz"
}
, Review.Test.error
{ message = "Detected bar"
, details = details
, under = "bar"
}
, Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
]
, test "should match multiple errors with the expected errors that are in a 'random' order" <|
\() ->
"""module MyModule exposing (b)
a = foo
bar
baz
"""
|> Review.Test.run ruleWithDifferentMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Detected bar"
, details = details
, under = "bar"
}
, Review.Test.error
{ message = "Detected baz"
, details = details
, under = "baz"
}
, Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
]
, test "should match multiple errors with the same details and 'under' if they are in the correct order" <|
\() ->
"""module MyModule exposing (b)
a = foo
foo
foo
"""
|> Review.Test.run ruleWithDifferentMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
|> Review.Test.atExactly { start = { row = 2, column = 5 }, end = { row = 2, column = 8 } }
, Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
|> Review.Test.atExactly { start = { row = 3, column = 5 }, end = { row = 3, column = 8 } }
, Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 8 } }
]
, test "should match multiple errors with the same details and 'under' if they are in a random order" <|
\() ->
"""module MyModule exposing (b)
a = foo
foo
foo
"""
|> Review.Test.run ruleWithDifferentMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
|> Review.Test.atExactly { start = { row = 3, column = 5 }, end = { row = 3, column = 8 } }
, Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 8 } }
, Review.Test.error
{ message = "Detected foo"
, details = details
, under = "foo"
}
|> Review.Test.atExactly { start = { row = 2, column = 5 }, end = { row = 2, column = 8 } }
]
]
sameMessageTest : Test
sameMessageTest =
describe "when message is the same for every error"
[ test "should match a single error to the single expected error" <|
\() ->
"""module MyModule exposing (b)
a = foo"""
|> Review.Test.run ruleWithSameMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "message"
, details = details
, under = "foo"
}
]
, test "should match multiple errors with the expected errors that are already in the correct order" <|
\() ->
"""module MyModule exposing (b)
a = foo
bar
baz
"""
|> Review.Test.run ruleWithSameMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "message"
, details = details
, under = "foo"
}
, Review.Test.error
{ message = "message"
, details = details
, under = "bar"
}
, Review.Test.error
{ message = "message"
, details = details
, under = "baz"
}
]
, test "should match multiple errors with the expected errors that are in reverse order" <|
\() ->
"""module MyModule exposing (b)
a = foo
bar
baz
"""
|> Review.Test.run ruleWithSameMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "message"
, details = details
, under = "baz"
}
, Review.Test.error
{ message = "message"
, details = details
, under = "bar"
}
, Review.Test.error
{ message = "message"
, details = details
, under = "foo"
}
]
, test "should match multiple errors with the expected errors that are in a 'random' order" <|
\() ->
"""module MyModule exposing (b)
a = foo
bar
baz
"""
|> Review.Test.run ruleWithSameMessage
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "message"
, details = details
, under = "bar"
}
, Review.Test.error
{ message = "message"
, details = details
, under = "baz"
}
, Review.Test.error
{ message = "message"
, details = details
, under = "foo"
}
]
]
-- TODO with different locations for the same element (with atexactly)
-- TEST RULES
ruleWithDifferentMessage : Rule
ruleWithDifferentMessage =
Rule.newModuleRuleSchema "NoValues" ()
|> Rule.withSimpleExpressionVisitor (expressionVisitor (\fnName -> "Detected " ++ fnName))
|> Rule.fromModuleRuleSchema
ruleWithSameMessage : Rule
ruleWithSameMessage =
Rule.newModuleRuleSchema "NoValues" ()
|> Rule.withSimpleExpressionVisitor (expressionVisitor (always "message"))
|> Rule.fromModuleRuleSchema
expressionVisitor : (String -> String) -> Node Expression -> List (Error {})
expressionVisitor messageFn node =
case Node.value node of
Expression.FunctionOrValue _ fnName ->
[ Rule.error
{ message = messageFn fnName
, details = details
}
(Node.range node)
]
_ ->
[]
details : List String
details =
[ "details" ]