You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "matthiasblaesing (via GitHub)" <gi...@apache.org> on 2025/01/24 17:59:04 UTC
[PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
matthiasblaesing opened a new pull request, #8187:
URL: https://github.com/apache/netbeans/pull/8187
For the groovy.editor unittests several fails were observed, where the testfailure did not result from the test itself, but by the cleanup routine. The cleanup removes the work directory to prepare for the next test run.
The recursive deletion can fail with an `IOException`, because new files are created, while the directory tree is traversed. Observed where `archives.properties` and `segments` from parsing API. Both are saved delayed.
Instead of failing, the deletion is retried to continue testing.
The observed stacktrace:
```
at java.base/sun.nio.fs.UnixFileSystemProvider.implDelete(UnixFileSystemProvider.java:246) at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105) at java.base/java.nio.file.Files.delete(Files.java:1152) at org.netbeans.junit.NbTestCase$4.postVisitDirectory(NbTestCase.java:1006) at org.netbeans.junit.NbTestCase$4.postVisitDirectory(NbTestCase.java:998) at java.base/java.nio.file.Files.walkFileTree(Files.java:2828) at java.base/java.nio.file.Files.walkFileTree(Files.java:2882) at org.netbeans.junit.NbTestCase.deleteFile(NbTestCase.java:998) at org.netbeans.junit.NbTestCase.deleteSubFiles(NbTestCase.java:1017) at org.netbeans.junit.NbTestCase.clearWorkDir(NbTestCase.java:1031) at org.netbeans.modules.groovy.editor.api.parser.GroovyParserTest.tearDown(GroovyParserTest.java:79) at org.netbeans.junit.NbTestCase.runBare(NbTestCase.java:530) at org.netbeans.junit.NbTestCase.run(NbTestCase.java:298)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8187:
URL: https://github.com/apache/netbeans/pull/8187#issuecomment-2613780545
@matthiasblaesing concurrent write is no good sign even during testing. The lucene index is built while the dir is cleared - this could cause all kinds of exceptions in the log etc.
some observations: no other groovy test clears the workdir on teardown, only on setup - lets do the same in `GroovyParserTest` (https://github.com/apache/netbeans/pull/8191).
If this would be just about the groovy tests it would be less of a problem, but `NbTestCase` is used everywhere. Knowing when concurrent writes during recursive delete happen can be useful information.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8187:
URL: https://github.com/apache/netbeans/pull/8187#issuecomment-2614032506
thats unfortunate - I would have liked to know why my assessment is wrong
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8187:
URL: https://github.com/apache/netbeans/pull/8187#issuecomment-2614023614
> I question that a failing cleanup is a good test indicator.
@matthiasblaesing the problem is that the method is not exclusively called during cleanup and if cleanup fails, tests shouldn't run. Iterating on cleanup does not assert that the folder is empty once it finishes, the concurrent write is outside of the control of that method. Having half-cleaned up folders is an even worse race condition in the making. Detecting concurrent writes is a feature not a bug in my opinion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #8187:
URL: https://github.com/apache/netbeans/pull/8187#issuecomment-2613818799
I question that a failing _cleanup_ is a good test indicator. For me this is a brittle test setup. Anyway, you made your point, so I'll step away and lets you fix the tests in the manor you prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #8187:
URL: https://github.com/apache/netbeans/pull/8187#issuecomment-2614028858
I won't start a discussion why I think this assessment is wrong, I made my point and will stop wasting my time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing closed pull request #8187: Tests: Add retries to NbTestCase#deleteFile to stabilize unittests
URL: https://github.com/apache/netbeans/pull/8187
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8187:
URL: https://github.com/apache/netbeans/pull/8187#issuecomment-2613229010
if something is still writing to the folder while the test is trying to delete it, the test probably should have to make sure first that this task is stopped.
Iterating on delete until the directory is gone does hide the problem, since the next test case could end up seeing unexpected files still.
If this is specific to the groovy tests, lets give the groovy tests their own delete method - the retry loop shouldn't run everywhere IMO since it can hide more problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Tests: Add retries to NbTestCase#deleteFile to stabilize unittests [netbeans]
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #8187:
URL: https://github.com/apache/netbeans/pull/8187#issuecomment-2613245154
What I see in the directories are the files created by indexing. Sure, I can mess with the production code to make these stoppable, but if retrying deletion in only test code, I think that is the better idea.
> if something is still writing to the folder while the test is trying to delete it, the test probably should have to make sure first that this task is stopped.
Sure, you just need to wait. At some point the parsing/scanning infrastructure will settle and you can delete.
> Iterating on delete until the directory is gone does hide the problem, since the next test case could end up seeing unexpected files still.
>
> If this is specific to the groovy tests, lets give the groovy tests their own delete method - the retry loop shouldn't run everywhere IMO since it can hide more problems.
This is the _cleanup_ code. _Not_ the test code. It is best effort and if it improves the situation.
Take it leave it, I'm willing to step away from this and close it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists