Support xmllint autofix

[bdovaz]

Context #2240

It will only work if XML_XMLLINT_CLI_LINT_MODE: file because the command needs an --output file which can only be passed per file.

I also created 2 files for the tests because the linter was of type list_of_files but only had one file of each.

@nvuillam I have doubts in this case and in the powershell one and it is that in all linter tests the autofix are never tested, no? I mean, there should be a test that checks the original content of a file and the expected one to see if it has been formatted correctly. Right now without those tests, we don't know if it really works or not.

[codecov-commenter]

Codecov Report

Merging #2244 (a3ff63f) into main (1d74fa0) will increase coverage by 0.46%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main    #2244      +/-   ##
==========================================
+ Coverage   82.60%   83.07%   +0.46%     
==========================================
  Files         170      171       +1     
  Lines        4484     4496      +12     
==========================================
+ Hits         3704     3735      +31     
+ Misses        780      761      -19     

Impacted Files	Coverage Δ
megalinter/linters/XmlLintLinter.py	66.66% <66.66%> (ø)
megalinter/reporters/UpdatedSourcesReporter.py	89.74% <0.00%> (+2.56%)	⬆️
...alinter/tests/test_megalinter/helpers/utilstest.py	89.01% <0.00%> (+8.05%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[nvuillam]

plz check comments :)

[nvuillam]

https://github.com/oxsecurity/megalinter/blob/main/megalinter/tests/test_megalinter/mega_linter_2_fixes_test.py

You can test the fix here :)

[bdovaz]

https://github.com/oxsecurity/megalinter/blob/main/megalinter/tests/test_megalinter/mega_linter_2_fixes_test.py

You can test the fix here :)

@nvuillam But what exactly do you mean?

I see that test_1_apply_fixes_on_one_linter is a JavaScript only test.

I see that test_2_apply_fixes_on_all_linters is a test that mixes everything, JavaScript, several different files in fixable_files...

Where do I have to add what? They are a bit messy those tests.

[nvuillam]

Yep it's messy but except you want to implement generic testing of fixing (that is an option but would require core enhancements & probably more test files), this classe is the only thing testing fixes today :/

[bdovaz]

@nvuillam please check comment replies...

About the tests you mention... For them to run correctly locally (on my machine) I have to have all the linters installed on my machine? I see that some of them work and others fail and I understand that is why, isn't it? How can I see the complete log? Because in VSCode I get the output but I see it like this, is it normal?

[nvuillam]

Hmmm indeed, run this test class locally can be tricky if all the linters locally installed are not running the same on windows & linux...

If your branch was directly on the repo and not from a fork, CI generates and push the docker image named like the branch, so it can be used for tests on another sample repo

Probably someday we should implement a generic way to test fixes are applied, like we do for success, failure, version, help and sarif... (but i'm afraid it makes the CI jobs even more longer :/ )

[nvuillam]

ok once CI is ok :)

[Kurt-von-Laven]

Hypothesis is a phenomenal property-based testing library for Python, but we're a ways off from that today.

[echoix]

Hypothesis is a phenomenal property-based testing library for Python, but we're a ways off from that today.

But we are testing that our wrapping code and calling the linter works, not testing the linter itself. As everyone, learning to better QA software is always a good thing. It's always a weak spot.

[Kurt-von-Laven]

Hypothesis can help with that too, but it's rarely the case that one would want to write all tests exclusively in Hypothesis. Property-based testing is intended to be used in conjunction with conventional example-based testing.

[bdovaz]

We take the testing conversation to #2250

If the PR is ok @nvuillam you can merge it