Avoid deprecation warnings and other minor cleanups in FileUtilsTest

[elharo]

@garydgregory

[garydgregory]

-1 as is, deprecated code must still be tested.

I guess we could have separate methods or classes that only test deprecated code but that seems like busy work.

It is OK to change 'main' code to not use deprecated elements, as long as test coverage does not go down, in which case a new test should be written to cover that change, for example if a deprecated method ends up not called at all from anywhere.

[elharo]

None of the deprecation changes affect the tests. They are changes to the setup code for the tests. More specifically it specifies a charset when writing initial data into a file that is overwritten, and the test is that the new data overwrites the old data.

This does identify a few places where we've failed to deprecate/provide alternative for some methods in the model code in the writeLines methods that don't take character sets.

[elharo]

There's one failure on Java 21 only. This test wasn't touched in this PR. It's probably just flaky, but it's also possible it's broken at HEAD:

Error: Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.178 s <<< FAILURE! -- in org.apache.commons.io.FileUtilsWaitForTest
Error: org.apache.commons.io.FileUtilsWaitForTest.testWaitForInterrupted -- Time elapsed: 0.022 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: but was:
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:31)
at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:183)
at org.apache.commons.io.FileUtilsWaitForTest.testWaitForInterrupted(FileUtilsWaitForTest.java:60)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

[elharo]

As far as code coverage I see

[2024-01-23T00:44:13.800Z] ['error'] There was an error running the uploader: Error uploading to https://codecov.io: Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}

[garydgregory]

Hi @elharo

None of the deprecation changes affect the tests.

But they do since some of the changes are to @Test methods.

I really don't want to fiddle with this type of change. If we want to create special tests that only test deprecated code (maybe using the testDeprecated method prefix), that's fine, then we can only test non-deprecated code in "normal" tests. Otherwise, I don't think we need to change @Test methods.

[elharo]

Yes, they are in @test methods but no, they are not tested by these methods. All the ones I have changed here do is write some data into a file that is then overwritten before anything is asserted. The purpose is to make sure that data is overwritten by the subsequent calls when append=False, not to test that the deprecated method is behaving according to spec.

[garydgregory]

@elharo I do not see any negative impact on the JaCoCo code coverage report for the FileUtils class or the whole code base. Merging.