save-cli icon indicating copy to clipboard operation
save-cli copied to clipboard

Tests from `examples/kotlin-diktat/warn-dir` are failing on Windows with diktat 1.1.0

Open orchestr7 opened this issue 3 years ago • 8 comments
trafficstars

Probably we will need to delete all tests and simply test real scenarios on the executable save-cli

orchestr7 avatar Jun 21 '22 09:06 orchestr7

Do you mean WarnDirTest? As I can see from the latest CI run, this is the test that passes on Linux and fails on Windows

Looks like something is broken in ktlint's pattern matching. save-cli runs the following command:

java -jar ktlint --disabled_rules=standard -R diktat.jar  C:\Users\<user>\AppData\Local\Temp\WarnPlugin--1261806134\warn-dir\**\*.kt

which matches no files. I tried to experiment with forward slashes, but no luck. This, however

 java -jar ktlint --disabled_rules=standard --debug -R diktat.jar  C:\Users\<user>\AppData\Local\Temp\WarnPlugin--1261806134\warn-dir\chapter1\*.kt

works as expected.

Also, running tests by relative glob also works:

...\save\examples\kotlin-diktat> java -jar ktlint --disabled_rules=standard --debug -R diktat.jar warn-dir\**\*.kt

petertrr avatar Jun 23 '22 14:06 petertrr

Only on windows

orchestr7 avatar Jun 23 '22 14:06 orchestr7

I've opened https://github.com/saveourtool/diktat/issues/1397 to investigate root cause and maybe even fix on ktlint's side. I'll disable these tests for now until this issue is resolved

petertrr avatar Jun 23 '22 14:06 petertrr

Actually now that I investigated it a bit, I find behavior of wildCardInDirectoryMode quite confusing. Why, for example, for warn-dir/chapter1 we are passing pattern to the tested tool as warn-dir/**/*.kt instead of warn-dir/chapter1/**/*.kt? Here is the log:

[TRACE]: Discovered the following test resources: [Test(test=../examples/kotlin-diktat/warn-dir/chapter1/EnumValueSnakeCaseTest.kt), Test(test=../examples/kotlin-diktat/warn-dir/chapter1/GenericFunctionTest.kt), Test(test=../examples/kotlin-diktat/warn-dir/chapter1/SmallTest.kt)]
[DEBUG]: Validated plugin configuration for [../examples/kotlin-diktat/warn-dir/chapter1/save.toml] ([WARN])
...
[TRACE]: Constructed file names for execution for warn plugin: C:\Users\<user>\AppData\Local\Temp\WarnPlugin--89925833\warn-dir/**/*.kt
[..
[DEBUG]: Executing: CMD, /C, cd /d ../examples/kotlin-diktat && java -jar ktlint --disabled_rules=standard -R diktat.jar  C:\Users\<user>\AppData\Local\Temp\WarnPlugin--89925833\warn-dir/**/*.kt with timeout 90000 ms

I think we should append wildcard pattern to the root of test suite that is being currently executed.

Edit: this is because of this code: https://github.com/saveourtool/save-cli/blob/1c2499512ace8bfb4167d8f4b865bbe237315b0c/save-common/src/commonMain/kotlin/com/saveourtool/save/core/utils/CmdExecutorBase.kt#L74-L78

Edit 2: this logic even is fixed in this test: https://github.com/saveourtool/save-cli/blob/1c2499512ace8bfb4167d8f4b865bbe237315b0c/save-core/src/commonNonJsTest/kotlin/com/saveourtool/save/core/integration/WarnDirTest.kt#L29-L35 but I can't neither remember nor understand why we implemented it this way :( @akuleshov7, do you remember if it's related to #350?

petertrr avatar Jun 23 '22 16:06 petertrr

As we discussed:

  1. [warn] plugin -> is simply processing separate files
  2. [warn] plugin + DIRECTORY flag -> run ONLY on this directory. Plugin uses from one single file with all warnings (no inline warnings). It will make the logic less complex. In this mode (with directory flag) we will ignore other nested save.toml files and detect tests from the file with expected warnings.

We need to create a logic on save-cloud for it also...

orchestr7 avatar Jun 24 '22 13:06 orchestr7

To sum up, tests in save-cli are failing because of some issues with ktlint 0.45.x + diktat 1.1.x, as described in https://github.com/saveourtool/save-cli/issues/402#issuecomment-1164474849. Let's create a separate issue for new logic of directory mode.

petertrr avatar Jun 24 '22 14:06 petertrr

Need to check the relevance of the problem, a lot of time have passed, and the versions of used tool were upgraded

kgevorkyan avatar Jan 10 '23 12:01 kgevorkyan

We should try to re-enable tests disabled in #407 and see if they still fail. Otherwise, the problem is indeed resolved.

petertrr avatar Jan 11 '23 08:01 petertrr