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

[IO-856] test for background deletion #697

Closed
wants to merge 19 commits into from
Closed

Conversation

thhart
Copy link
Contributor

@thhart thhart commented Nov 5, 2024

test for exceptional listing of deleted files

See IO-856

Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @thhart

Thank you for your PR.

Please see my comments.

# Conflicts:
#	src/test/java/org/apache/commons/io/FileUtilsListFilesTest.java
Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thhart

Thank you for your update.

You did not apply my comments throughout. See my additional comments.

TY

Copy link
Member

@garydgregory garydgregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thhart

Please see my comments, same as before ;-)

@garydgregory
Copy link
Member

Hello @thhart

Thank you for your update.

Interesting that the build on Java 24-ea is green.

Ubuntu shows:

FileUtilsListFilesTest.setUp:77 » FileSystem /tmp/junit-6966172520746092400/dummy-build.xml: Too many open files

Is that the point of the test?

Another builds show:

[INFO] There are 5 errors reported by Checkstyle 9.3 with D:\a\commons-io\commons-io/src/conf/checkstyle.xml ruleset.
Error:  src\test\java\org\apache\commons\io\FileUtilsListFilesTest.java:[27,1] (imports) ImportOrder: Wrong order for 'java.io.BufferedOutputStream' import.
Error:  src\test\java\org\apache\commons\io\FileUtilsListFilesTest.java:[41,1] (imports) ImportOrder: Wrong order for 'org.junit.jupiter.api.Assertions.assertEquals' import.
Error:  src\test\java\org\apache\commons\io\FileUtilsListFilesTest.java:[220,9] (whitespace) WhitespaceAfter: 'if' is not followed by whitespace.
Error:  src\test\java\org\apache\commons\io\FileUtilsListFilesTest.java:[245,18] (coding) FinalLocalVariable: Variable 'endTime' should be declared final.
Error:  src\test\java\org\apache\commons\io\FileUtilsListFilesTest.java:[261,18] (coding) FinalLocalVariable: Variable 'endTime' should be declared final.

Please run mvn by itself to run all build checks and fix errors before you push.

@thhart
Copy link
Contributor Author

thhart commented Nov 8, 2024

FileUtilsListFilesTest.setUp:77 » FileSystem /tmp/junit-6966172520746092400/dummy-build.xml: Too many open files


Is that the point of the test?

No, this is not failing in my environment. Maybe your Ubuntu is too restrictive in this.

[INFO] There are 5 errors reported by Checkstyle 9.3 with D:\a\commons-io\commons-io/src/conf/checkstyle.xml ruleset.

This is changed now.

Please run mvn by itself to run all build checks and fix errors before you push.

[ERROR] Errors: 
[ERROR]   FileUtilsListFilesTest.testListFilesWithDeletionThreaded:273 » Execution org.opentest4j.AssertionFailedError: this should not happen
[INFO] 
[ERROR] Tests run: 4246, Failures: 0, Errors: 1, Skipped: 24
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  05:18 min
[INFO] Finished at: 2024-11-08T09:09:46+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.1:test (default-test) on project commons-io: 
[ERROR] 
[ERROR] Please refer to /home/th/dev/os/commons-io/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.

This fails in my Ubuntu system and in other Linux flavour. Can not say for other systems.

@garydgregory
Copy link
Member

@thhart

Thank you for your updates. I'll take a look over the next couple of days.

@garydgregory
Copy link
Member

Interesting: This is not reproducible on Windows in this CI for Java 8, 11, 17, 21, and 23.

@garydgregory
Copy link
Member

Interesting meta-failure on Ubuntu:

[INFO] Running org.apache.commons.io.FileUtilsListFilesTest
*** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with message can't create byte arrau at JPLISAgent.c line: 813
*** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with message can't create byte arrau at JPLISAgent.c line: 813
*** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with message can't create byte arrau at JPLISAgent.c line: 813
*** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with message can't create byte arrau at JPLISAgent.c line: 813

@garydgregory
Copy link
Member

Hello @thhart

  • I added a slightly modified version of this test in FileUtilsListFilesTest.testListFilesWithDeletionThreaded()
  • This test is only enabled on Windows
  • There are hidden failures on GitHub CI on Ubuntu and macOS

@garydgregory
Copy link
Member

@thhart

Please test git master. Closing this PR in favor or #699.

@thhart
Copy link
Contributor Author

thhart commented Nov 11, 2024

Looks ok now, have no problems any more.
(There was a wrong escape in the extension, it is not a pattern. I created a small patch for it.)

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