[
https://issues.apache.org/jira/browse/GROOVY-12219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18100436#comment-18100436
]
ASF GitHub Bot commented on GROOVY-12219:
-----------------------------------------
testlens-app[bot] commented on PR #2753:
URL: https://github.com/apache/groovy/pull/2753#issuecomment-5131489816
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them
below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [Build and test / lts \(17, windows-latest,
1\)](https://github.com/apache/groovy/actions/runs/30543052468/job/90872205949?pr=2753)
> :test
| Test | Runs | Flakiness |
|---|---|--:|
| ClassTagExtensionModuleTest | ❌ | 23% 🔴 |
🏷️ Commit: 610074682a008855f853b261c9660ca30f882f7b
▶️ Tests: 94240 executed
🟡 Checks: 28/30 completed
### Test Failures
<details>
<summary><strong>ClassTagExtensionModuleTest</strong> (:test in <a
href="https://github.com/apache/groovy/actions/runs/30543052468/job/90872205949?pr=2753";>Build
and test / lts (17, windows-latest, 1)</a>)</summary>
```
org.junit.platform.commons.JUnitException: Failed to close extension context
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by:
org.junit.jupiter.api.io.TempDirDeletionStrategy$DeletionException: Failed to
delete temp directory
C:\Users\RUNNER~1\AppData\Local\Temp\junit-14201149565700683247. The following
paths could not be deleted (see suppressed exceptions for details): <root>,
classtag-module-test.jar
Suppressed: java.nio.file.DirectoryNotEmptyException:
C:\Users\RUNNER~1\AppData\Local\Temp\junit-14201149565700683247
at
java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:272)
at
java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
at java.base/java.nio.file.Files.delete(Files.java:1152)
at java.base/java.nio.file.Files.walkFileTree(Files.java:2828)
at java.base/java.nio.file.Files.walkFileTree(Files.java:2882)
at
java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at
java.base/java.util.stream.SortedOps$RefSortingSink.end(SortedOps.java:395)
at
java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
at
java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
at
java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510)
at
java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at
java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
at
java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
at
java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at
java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Suppressed: java.nio.file.FileSystemException:
C:\Users\RUNNER~1\AppData\Local\Temp\junit-14201149565700683247\classtag-module-test.jar:
The process cannot access the file because it is being used by another process
at
java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:92)
at
java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103)
at
java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:108)
at
java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:275)
at
java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
at java.base/java.nio.file.Files.delete(Files.java:1152)
at java.base/java.nio.file.Files.walkFileTree(Files.java:2811)
at java.base/java.nio.file.Files.walkFileTree(Files.java:2882)
at
java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at
java.base/java.util.stream.SortedOps$RefSortingSink.end(SortedOps.java:395)
at
java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
at
java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
at
java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510)
at
java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at
java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
at
java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
at
java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at
java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Suppressed: java.nio.file.FileSystemException:
C:\Users\RUNNER~1\AppData\Local\Temp\junit-14201149565700683247\classtag-module-test.jar:
The process cannot access the file because it is being used by another process
... 20 more
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 ClassTagExtensionModuleTest <!
> Lazy iterators that prefetch remove the wrong element in DefaultGroovyMethods
> -----------------------------------------------------------------------------
>
> Key: GROOVY-12219
> URL: https://issues.apache.org/jira/browse/GROOVY-12219
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> h3. Summary
> Several lazy {{Iterator}} implementations in {{DefaultGroovyMethods}}
> *prefetch* the next element from their delegate (inside {{advance()}}, which
> {{next()}} calls before returning), but implement {{remove()}} by delegating
> to {{delegate.remove()}}. Because the delegate's cursor has already advanced
> past the just-yielded element, {{delegate.remove()}} removes the *wrong*
> element (the one prefetched ahead) rather than the element the caller last
> obtained from {{next()}}.
> This was found and fixed for {{TakeWhileIterator}} under GROOVY-12215 (its
> {{remove()}} now throws {{UnsupportedOperationException}}). The same pattern
> exists in other iterators in the file and should be audited and fixed
> consistently.
> h3. The pattern
> {code:java}
> public E next() {
> if (exhausted) throw new NoSuchElementException();
> E result = next; // element yielded to the caller
> advance(); // calls delegate.next() -> prefetches the NEXT
> element
> return result;
> }
> public void remove() {
> if (exhausted) throw new NoSuchElementException();
> delegate.remove(); // removes the prefetched-ahead element, not `result`
> }
> {code}
> h3. Affected iterators (private static classes in DefaultGroovyMethods)
> ||Iterator||Prefetches ahead?||{{remove()}}||Status||
> |{{TakeWhileIterator}}|yes ({{result = next; advance()}})|delegated
> (buggy)|*Fixed* in GROOVY-12215 — now throws
> {{UnsupportedOperationException}}|
> |{{ToUniqueIterator}}|yes ({{result = next; advance()}})|delegated|*Suspect*
> — same pattern; verify and fix|
> |{{DropRightIterator}}|yes (reads ahead into a {{discards}}
> buffer)|delegated|*Suspect* — reads ahead by N; verify and fix|
> |{{DropWhileIterator}}|no (no steady-state prefetch)|delegated|OK — removes
> the correct element|
> |{{CollectManyIterator}}, {{CollectManyFunctionIterator}},
> {{InjectAllIterator}}|n/a|throws {{UnsupportedOperationException}}|OK —
> already unsupported|
> h3. Reproduction (to confirm per iterator)
> {code:groovy}
> def source = new LinkedList([1, 1, 2, 3]) // mutable, supports
> Iterator.remove()
> def it = source.iterator().toUnique()
> assert it.next() == 1
> it.remove() // intends to remove the yielded 1
> // the delegate has already advanced past it, so a later element is removed
> instead
> {code}
> h3. Proposed fix
> Make {{remove()}} throw {{UnsupportedOperationException}} for the prefetching
> iterators ({{ToUniqueIterator}}, {{DropRightIterator}}), matching the fix
> applied to {{TakeWhileIterator}} and the existing behaviour of
> {{CollectManyIterator}}/{{InjectAllIterator}}. (Reworking them to avoid
> prefetching before {{remove()}} is possible but more invasive; making
> {{remove()}} unsupported is the simplest correct option and is consistent
> with the other lazy iterators.) Leave {{DropWhileIterator}} as-is.
> h3. Notes
> * No test currently exercises {{remove()}} on these iterators, so the change
> is low risk.
> * Turning a silently-wrong {{remove()}} into an explicit
> {{UnsupportedOperationException}} is strictly safer than the current
> behaviour.
> * Discovered while addressing a Copilot review comment on GROOVY-12215.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
