Add known broken tests for import placement #2425
Conversation
ghcide/test/exe/Main.hs
Outdated
| @@ -855,7 +855,18 @@ watchedFilesTests = testGroup "watched files" | |||
|
|
|||
| insertImportTests :: TestTree | |||
| insertImportTests = testGroup "insert import" | |||
| [ checkImport "above comment at top of module" "CommentAtTop.hs" "CommentAtTop.expected.hs" "import Data.Monoid" | |||
| [ (`xfail` "known broken") (checkImport "module where keyword lower in file no exports" "WhereKeywordLowerInFileNoExports.hs" "WhereKeywordLowerInFileNoExports.expected.hs" "import Data.Int") | |||
There was a problem hiding this comment.
please could you use the expectFailBecause existing function, if possible with a short insight about why do you think it fail ("known broken" would be fine though)
There was a problem hiding this comment.
Sure, I'll do that later today.
|
Many thanks for adding tests, they will help a lot in an eventual resolution //cc @eddiemundo |
nini-faroux
force-pushed
the
import-tests
branch
from
93f67d8 to
ea3883a
Compare
| {-# OPTIONS_GHC -Wall, | ||
| OPTIONS_GHC -Wno-unrecognised-pragmas #-} |
There was a problem hiding this comment.
The expected version of this test doesn't have the OPTIONS_GHC -Wno-unrecognized-pragmas line. I think the expected version of the test should have this line unless the test does more than add an import.
There was a problem hiding this comment.
no you're right, that's updated now.
|
I see what @nini-faroux was talking about on where to extract the "pre-decl" parser functionality since it's needed in both ghcide CodeAction.hs, and the pragmas plugin. I also need it in the hlint plugin. Of the existing places I think putting it in its own module under ghcide/src/Development/IDE/Spans would be ok since Spans is vaguely related, but stuffing it into CodeAction.hs or in its own module in that folder seems ok too since it has random stuff in there. I think I might extract that functionality into one of those places later today. |
nini-faroux
force-pushed
the
import-tests
branch
from
ea3883a to
3762375
Compare
ghcide/test/exe/Main.hs
Outdated
| [ expectFailBecause "'findPositionFromImportsOrModuleDecl' function adds import directly under line with module declaration, not accounting for case when 'where' keyword is placed on lower line" | ||
| (checkImport "module where keyword lower in file no exports" "WhereKeywordLowerInFileNoExports.hs" "WhereKeywordLowerInFileNoExports.expected.hs" "import Data.Int") |
There was a problem hiding this comment.
minor formatting thing, maybe the tests could be more legible if they are splitted in several lines, something like
[ expectFailBecause
$ "'findPositionFromImportsOrModuleDecl' function adds import directly under line with module declaration, "
++ "not accounting for case when 'where' keyword is placed on lower line"
(checkImport
"module where keyword lower in file no exports"
"WhereKeywordLowerInFileNoExports.hs"
"WhereKeywordLowerInFileNoExports.expected.hs"
"import Data.Int")
There was a problem hiding this comment.
that's true, they're updated now
nini-faroux
force-pushed
the
import-tests
branch
from
3762375 to
1f0d31f
Compare
Co-authored-by: Javier Neira <[email protected]>
Tests for known issues with import placement.