Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with expecting default ignore file when checking file size #545

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lnenad
Copy link
Contributor

@lnenad lnenad commented Sep 30, 2024

This PR fixes issue with expecting default ignore file when checking file size and adds case to check custom ignore file size. The issue was made by my previous PR #543 due to the fact it was always trying to File.stat! the default ignore file.

I've manually tested the following 5 combinations:

  • no ignore file at all no config
  • default ignore file no config not empty
  • default ignore file no config empty
  • non-default ignore file path in config not empty
  • non-default ignore file path in config empty

They all work but please @jeremyjh take a look. I'd also be happy to write tests for this but for some reason tests are failing even if I go back in history before my previous PR, can you please take a look if it is failing for you? Maybe it's something with my setup.

…and adds case to check custom ignore file size
@jeremyjh
Copy link
Owner

@lnenad the tests do all pass for me locally. Can you paste a failure message? Yes I'd prefer to have tests and I think it should be possible following our project fixture patterns.

@lnenad
Copy link
Contributor Author

lnenad commented Sep 30, 2024

@jeremyjh here is the output

OS: Windows

elixir --version
Erlang/OTP 25 [erts-13.2.2.10] [source] [64-bit] [smp:14:14] [ds:14:14:10] [async-threads:1] [jit:ns]       

Elixir 1.17.3 (compiled with Erlang/OTP 25)

Ran like this: git clone repo, mix deps.get, mix test

PS mix test
  1) test Filtered dialyzer warnings (Dialyxir.ProjectTest)
     test/dialyxir/project_test.exs:147
     Assertion with == failed
     code:  assert lines == ["project.ex:9 This should still be here"]
     left:  ["project.ex:9 This should still be here",
             "project.ex:9: Guard test is_atom(_@5::\#{'__exception__':='true', '__struct__':=_, _=>_}) can never succeed",
             "project.ex:9: Guard test is_binary(_@4::\#{'__exception__':='true', '__struct__':=_, _=>_}) can never succeed"]
     right: ["project.ex:9 This should still be here"]
     stacktrace:
       (mix 1.17.3) lib/mix/project.ex:463: Mix.Project.in_project/4    
       (elixir 1.17.3) lib/file.ex:1665: File.cd!/2
       test/dialyxir/project_test.exs:148: (test)

==> dialyxir
Total errors: 1, Skipped: 0, Unnecessary Skips: 2
.

  2) test listing unused filter behaves the same for different formats (Dialyxir.FormatterTest)
     test/dialyxir/formatter_test.exs:195
     Assertion with == failed       
     code:  assert unused == expected_unused_filter
     left:  "Unused filters:\n{\"lib/short_description.ex:17:no_return Function format_long/1 has no local return.\"}\n{\"lib/file/warning_type.ex\", :no_return, 18}\n{\"lib/file/warning_type/line.ex\", :no_return, 19}"
     right: "Unused filters:\r\n{\"lib/short_description.ex:17:no_return Function format_long/1 has no local return.\"}\r\n{\"lib/file/warning_type.ex\", :no_return, 18}\r\n{\"lib/file/warning_type/line.ex\", :no_return, 19}"
     stacktrace:
       test/dialyxir/formatter_test.exs:218: anonymous fn/1 in Dialyxir.FormatterTest."test listing unused filter behaves the same for different formats"/1
       (ex_unit 1.17.3) lib/ex_unit/capture_io.ex:315: ExUnit.CaptureIO.do_capture_gl/2
       (ex_unit 1.17.3) lib/ex_unit/capture_io.ex:273: ExUnit.CaptureIO.do_with_io/3
       (ex_unit 1.17.3) lib/ex_unit/capture_io.ex:142: ExUnit.CaptureIO.capture_io/1
       (mix 1.17.3) lib/mix/project.ex:463: Mix.Project.in_project/4    
       (elixir 1.17.3) lib/file.ex:1665: File.cd!/2
       (elixir 1.17.3) lib/enum.ex:1703: Enum."-map/2-lists^map/1-1-"/2 
       test/dialyxir/formatter_test.exs:211: (test)

==> ignore
Total errors: 1, Skipped: 1, Unnecessary Skips: 3
.==> dialyxir
Total errors: 1, Skipped: 0, Unnecessary Skips: 2
.Total errors: 1, Skipped: 0, Unnecessary Skips: 2


  3) test formats dialyzer warning file location including column for Elixir.Dialyxir.Formatter.Dialyxir formatter (Dialyxir.FormatterTest)     
     test/dialyxir/formatter_test.exs:40
     Assertion with =~ failed       
     code:  assert message =~       
              "lib/file/warning_type/line.ex:19:4:no_return\nFunction format_long/1 has no local return."   
     left:  "lib/file/warning_type/line.ex:19:4:no_return\r\nFunction format_long/1 has no local return.\r\n________________________________________________________________________________"
     right: "lib/file/warning_type/line.ex:19:4:no_return\nFunction format_long/1 has no local return."     
     stacktrace:
       test/dialyxir/formatter_test.exs:52: (test)

Total errors: 1, Skipped: 0, Unnecessary Skips: 2
.==> ignore
Total errors: 1, Skipped: 1, Unnecessary Skips: 3
.==> ignore
Total errors: 1, Skipped: 1, Unnecessary Skips: 3
.==> ignore
Total errors: 3, Skipped: 3, Unnecessary Skips: 1
.==> dialyxir
Total errors: 1, Skipped: 0, Unnecessary Skips: 2
.Total errors: 1, Skipped: 0, Unnecessary Skips: 2
.==> ignore
Total errors: 1, Skipped: 1, Unnecessary Skips: 3
.==> ignore
Total errors: 1, Skipped: 0, Unnecessary Skips: 4
.

  4) test simple string ignore evaluates an ignore file and ignores warnings matching the pattern (Dialyxir.FormatterTest)
     test/dialyxir/formatter_test.exs:168
     ** (UndefinedFunctionError) function :dialyzer.format/1 is undefined or private. Did you mean:

           * format_warning/1       
           * format_warning/2       

     stacktrace:
       (dialyzer 5.0.5) :dialyzer.format({:warn_matching, {~c"a/file.ex", 17}, {:pattern_match, [~c"pattern 'ok'", ~c"'error'"]}})
       (elixir 1.17.3) lib/enum.ex:1703: Enum."-map/2-lists^map/1-1-"/2 
       (elixir 1.17.3) lib/enum.ex:4353: Enum.flat_map_list/2
       (dialyxir 1.4.3) lib/dialyxir/formatter.ex:42: Dialyxir.Formatter.format_and_filter/5
       test/dialyxir/formatter_test.exs:173: anonymous fn/0 in Dialyxir.FormatterTest."test simple string ignore evaluates an ignore file and ignores warnings matching the pattern"/1
       (mix 1.17.3) lib/mix/project.ex:463: Mix.Project.in_project/4    
       (elixir 1.17.3) lib/file.ex:1665: File.cd!/2
       test/dialyxir/formatter_test.exs:172: (test)

==> ignore_strict
Total errors: 3, Skipped: 3, Unnecessary Skips: 0
.==> dialyxir
Total errors: 1, Skipped: 0, Unnecessary Skips: 2
.==> ignore
Total errors: 1, Skipped: -1, Unnecessary Skips: 4
..............

  5) test check plts (Dialyxir.PltTest)
     test/dialyxir/plt_test.exs:9   
     Assertion with =~ failed       
     code:  assert capture_io(fun) =~
              "Looking up modules in dialyxir_erlang-20.3.plt\n" <>     
                "Looking up modules in dialyxir_erlang-20.3_elixir-1.6.2_deps-dev.plt\n" <>
                "/var/dialyxir_erlang-20.3_elixir-1.6.2_deps-dev.plt\n" <> "/var/dialyxir_erlang-20.3.plt\n"
     left:  "==> dialyxir\nLooking up modules in dialyxir_erlang-20.3.plt\nLooking up modules in dialyxir_erlang-20.3_elixir-1.6.2_deps-dev.plt\nc:/var/dialyxir_erlang-20.3_elixir-1.6.2_deps-dev.plt\nc:/var/dialyxir_erlang-20.3.plt\n"
     right: "Looking up modules in dialyxir_erlang-20.3.plt\nLooking up modules in dialyxir_erlang-20.3_elixir-1.6.2_deps-dev.plt\n/var/dialyxir_erlang-20.3_elixir-1.6.2_deps-dev.plt\n/var/dialyxir_erlang-20.3.plt\n"
     stacktrace:
       test/dialyxir/plt_test.exs:19: (test)


Finished in 1.7 seconds (0.00s async, 1.7s sync)
55 tests, 5 failures, 3 excluded, 1 skipped

Again, I am not proficient in Elixir so please tell me if the reporting of 5 failures is expected.

@jeremyjh
Copy link
Owner

@lnenad The issue with some of those is the line terminators are different on windows; its true the test suite isn't compatible with windows. I'm not sure what the story is with format vs format_warning though; if that is a platform difference it is a surprising one. Any chance you can test in WSL or a VM with Linux?

@jeremyjh
Copy link
Owner

jeremyjh commented Oct 1, 2024

@lnenad I think actually that those issues shouldn't prevent you from making a new test. If you can make a new test that passes for you in windows and fails with the old code, I can handle any issues with Linux compatibility/CI.

@lnenad
Copy link
Contributor Author

lnenad commented Oct 1, 2024

@jeremyjh Added tests, please take a look, they caused the CI to fail but they work for me locally. It's weird.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants