[
https://issues.apache.org/jira/browse/GROOVY-12343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110924#comment-18110924
]
ASF GitHub Bot commented on GROOVY-12343:
-----------------------------------------
testlens-app[bot] commented on PR #2867:
URL: https://github.com/apache/groovy/pull/2867#issuecomment-5519246747
## π¨ TestLens detected 3 failed tests π¨
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/33704943106/job/100491952032?pr=2867)
> :groovy-groovysh:test
| Test | Runs | Flakiness |
|---|---|--:|
| GroovyPosixCommandsTest >
headAndGrepAndWcStripControlCharactersFromNames\(\) | β | 24% π΄ |
| GroovyPosixCommandsTest > lsStripsControlCharactersFromFileNames\(\) | β |
24% π΄ |
| GroovyPosixCommandsTest > lsStripsControlCharactersFromSymlinkTargets\(\)
| β | 24% π΄ |
π·οΈ Commit: beaf2857ebb054e5dabe0502e809f0c09d6e9849
βΆοΈ Tests: 68915 executed
π‘ Checks: 8/29 completed
### Test Failures
<details><summary><strong>GroovyPosixCommandsTest >
headAndGrepAndWcStripControlCharactersFromNames()</strong>
(:groovy-groovysh:test in <a
href="https://github.com/apache/groovy/actions/runs/33704943106/job/100491952032?pr=2867";>Build
and test / lts (17, windows-latest, 1)</a>)</summary>
```
java.nio.file.InvalidPathException: Illegal char <> at index 2:
ok[2Ktrap.txt
at
java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
at
java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232)
at java.base/java.nio.file.Path.resolve(Path.java:515)
at
java.base/jdk.internal.reflect.GeneratedMethodAccessor31.invoke(Unknown Source)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at
org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:391)
at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:298)
at
org.codehaus.groovy.vmplugin.v8.IndyInterface.invokeColdReflective(IndyInterface.java:648)
at
org.apache.groovy.groovysh.jline.GroovyPosixCommandsTest.headAndGrepAndWcStripControlCharactersFromNames(GroovyPosixCommandsTest.groovy:123)
```
</details>
<details><summary><strong>GroovyPosixCommandsTest >
lsStripsControlCharactersFromFileNames()</strong> (:groovy-groovysh:test in <a
href="https://github.com/apache/groovy/actions/runs/33704943106/job/100491952032?pr=2867";>Build
and test / lts (17, windows-latest, 1)</a>)</summary>
```
java.nio.file.InvalidPathException: Illegal char <> at index 2:
ok[2Ktrap.txt
at
java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
at
java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232)
at java.base/java.nio.file.Path.resolve(Path.java:515)
at
java.base/jdk.internal.reflect.GeneratedMethodAccessor31.invoke(Unknown Source)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at
org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:391)
at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:298)
at
org.codehaus.groovy.vmplugin.v8.IndyInterface.invokeColdReflective(IndyInterface.java:648)
at
org.apache.groovy.groovysh.jline.GroovyPosixCommandsTest.lsStripsControlCharactersFromFileNames(GroovyPosixCommandsTest.groovy:97)
```
</details>
<details><summary><strong>GroovyPosixCommandsTest >
lsStripsControlCharactersFromSymlinkTargets()</strong> (:groovy-groovysh:test
in <a
href="https://github.com/apache/groovy/actions/runs/33704943106/job/100491952032?pr=2867";>Build
and test / lts (17, windows-latest, 1)</a>)</summary>
```
java.nio.file.InvalidPathException: Illegal char <> at index 2:
ok[2Ktrap.txt
at
java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
at
java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
at
java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232)
at java.base/java.nio.file.Path.resolve(Path.java:515)
at
java.base/jdk.internal.reflect.GeneratedMethodAccessor31.invoke(Unknown Source)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at
org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:391)
at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:298)
at
org.codehaus.groovy.vmplugin.v8.IndyInterface.invokeColdReflective(IndyInterface.java:648)
at
org.apache.groovy.groovysh.jline.GroovyPosixCommandsTest.lsStripsControlCharactersFromSymlinkTargets(GroovyPosixCommandsTest.groovy:108)
```
</details>
### Rerun Controls
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
π² GroovyPosixCommandsTest >
headAndGrepAndWcStripControlCharactersFromNames\(\) <!
> Provide AsyncChannel.after
> --------------------------
>
> Key: GROOVY-12343
> URL: https://issues.apache.org/jira/browse/GROOVY-12343
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> h2. Summary
> A {{ChannelSelect}} cannot have a timeout branch. Every comparable system has
> one β it is how "wait for
> work, but not forever" is written β and so, already, does the OTHER half of
> this library: an actor can
> schedule a message to itself, a select cannot wait on a deadline.
> There is a workaround, and it is worth being precise about how far it goes,
> because it goes further than it
> first appears.
> h2. What a user writes today
> {{Awaitable.orTimeout}} wraps the whole select:
> {code:groovy}
> try {
> def r = await alt.select().orTimeoutMillis(100)
> ...
> } catch (TimeoutException e) {
> ... // the timeout branch
> }
> {code}
> This is *safe*. {{AsyncSupport.completeOnTimeout}}/{{orTimeout}} cancel the
> source when the deadline fires,
> and {{Winner.cancel}} is {{claim.tryCommitCancel() && super.cancel(β¦)}}, so
> the cancellation wins the claim
> and no offer commits. GROOVY-12320's protocol already makes the outer timeout
> correct β nothing is consumed
> and lost. That is worth saying plainly, since it is the first thing a
> reviewer will worry about.
> What it does not give:
> * *Exceptions for expected control flow.* A timeout that is part of the
> design is signalled by
> {{TimeoutException}} rather than by a branch.
> * *No fallback form.* {{completeOnTimeout}} needs a fallback value, and
> {{ChannelSelect.Result}}'s
> constructor is package-private β a caller cannot construct one. So only the
> throwing form is usable here.
> * *One deadline for the whole select.* Different deadlines on different
> branches need nested selects or
> extra tasks.
> * *No guarded timeout.* The deadline sits outside the select, so {{Offer.when
> \{ β¦ \}}} (GROOVY-12326)
> cannot reach it. "Time out only while the buffer is empty" β never
> mid-transaction β is not expressible.
> * *No composition with the choice policy.* The timeout is not a branch, so it
> is outside {{fair()}} /
> {{random()}} rotation and outside the {{select(boolean...)}} mask.
> h2. What it enables
> Ordinary, and common:
> * wait for work, and on silence emit a heartbeat, retry, log, or shut down;
> * serve requests while a periodic tick flushes a batch, emits metrics, or
> polls for shutdown.
> Both are achievable today with try/catch. The two that are genuinely awkward
> are per-branch deadlines and
> the guarded timeout above.
> h2. The library is already inconsistent about this
> The strongest argument is not what other ecosystems do; it is that
> {{groovy.concurrent}} has already decided
> this question once, on the other side of the library. {{ActorContext}}
> carries:
> {code:java}
> Cancellable scheduleOnce(T message, java.time.Duration delay)
> Cancellable scheduleAtFixedRate(T message, java.time.Duration initial,
> java.time.Duration period)
> {code}
> So an actor can say "deliver me this in 100ms" and interleave it with the
> messages it is already handling β
> which is the actor spelling of exactly the shape a timer branch gives a
> select. The CSP half has no
> equivalent. {{AsyncChannel.after(β¦)}} is therefore not a new capability for
> the library so much as parity
> between its two halves, and the absence is more likely to read as an
> oversight than as a decision.
> The same asymmetry shows in the bounded forms: {{ActorOptions}} has
> {{withBoundedMailbox(n, Overflow)}} and
> {{withStashBound(n, StashOverflow)}}, so the actor side is already explicit
> about time and about bounds,
> while the channel side is explicit about neither.
> h2. Prior art β everyone has this
> || System || How the timeout appears || Timer is⦠||
> | Go | {{case <-time.After(d):}} | a channel |
> | Rust (crossbeam-channel) | {{recv(after(timeout)) -> _ =>}} | a channel
> ({{after(Duration) -> Receiver<Instant>}}) |
> | Clojure core.async | {{(alts! \[ch (timeout 1000)\])}} | a channel |
> | Kotlin coroutines | {{select \{ onTimeout(ms) \{ β¦ \} \}}} | a select
> clause |
> | Erlang | {{receive β¦ after Timeout ->}} | language syntax |
> | Ada | {{select β¦ or delay 5.0;}} | language syntax |
> | JCSP | {{CSTimer}} in the {{Guard\[\]}} | a guard kind |
> | *Groovy 6 today* | *β (outer wrapper only)* | *β* |
> Kotlin is the closest precedent for Groovy's situation: it has BOTH the outer
> wrapper
> ({{withTimeout}}/{{withTimeoutOrNull}}) and the branch ({{onTimeout}}).
> Having the wrapper is not treated as
> making the branch redundant.
> Note what Go, Rust and Clojure share: *the timer is a channel*, not a special
> branch kind. That shapes the
> proposal below.
> h2. Option A (preferred) β a timer CHANNEL
> {code:java}
> public static AsyncChannel<java.time.Instant> after(long millis)
> public static AsyncChannel<java.time.Instant> after(java.time.Duration
> duration)
> {code}
> A capacity-1 channel that delivers one element β the instant it fired β after
> the given delay.
> {code:groovy}
> def alt = ChannelSelect.from(work, AsyncChannel.after(100))
> def r = await alt.select()
> if (r.index == 1) { ... timed out ... }
> {code}
> *Why this one.* It needs *no change to {{ChannelSelect}} at all*, and it
> composes with everything already
> there for free: {{from(β¦)}} and {{offers(receive(β¦))}}, {{Offer.when \{ β¦
> \}}} for the guarded timeout,
> {{fair()}} / {{random()}} rotation, the {{select(boolean...)}} mask, and
> {{Result.getChannel()}} (which
> identifies the timer like any other branch, rather than having to return
> null). It is also useful *outside*
> a select β {{await AsyncChannel.after(50).first()}} as a delay, or as one arm
> of {{Awaitable.any}} β where a
> select-only offer would not reach. And it is the design three of the four
> comparable ecosystems chose.
> *Implementation.* A channel whose element is produced by one scheduled task,
> on the same scheduler
> {{AsyncSupport}} already uses for {{orTimeout}}. The single new concern is
> *cancellation*: when a select
> withdraws its losing branches the timer's {{ScheduledFuture}} should be
> cancelled too, or an abandoned
> select leaves a task pending until it fires. That is one thing to get right,
> against five for Option B.
> h2. Option B β a timer OFFER
> {code:java}
> public static ChannelSelect.Offer after(long millis)
> {code}
> Symmetric with the existing {{send(β¦)}} / {{receive(β¦)}} offers, and closest
> to JCSP's {{CSTimer}} guard.
> The cost is spread wider. {{Offer}} currently always has a channel, so it
> gains a nullable one plus a delay
> (or a subtype); {{Result.getChannel()}} must return null for a timer win, and
> say so; the registration loop
> in {{select(int\[\])}} gains a third path beside {{sendIfUnclaimed}} /
> {{receiveIfUnclaimed}} β though that
> path is largely written already, since a timer can compete for the claim
> exactly as a non-claimable foreign
> channel does, via {{winner.claim.tryCommit(future)}}. Withdrawal must still
> cancel the scheduled task.
> Option B is defensible if the symmetry with {{send}}/{{receive}} matters more
> than the reuse; it is listed
> so the choice is explicit rather than implied.
> h2. Open questions
> * *When does the clock start?* At creation, as in Go and crossbeam, is the
> obvious answer β and it means a
> fresh timer per loop iteration, so a long-running loop allocates one
> scheduled task per round. Go carries
> the same caveat for {{time.After}} in a loop and answers it with a
> resettable {{Timer}}. Worth deciding
> whether Groovy wants a resettable form now or later.
> * *Periodic ticks.* Go has {{time.Tick}} and crossbeam {{tick(d)}} β a
> channel that fires repeatedly. That
> is the natural companion for "every 100ms, flush", and is probably a
> separate issue rather than part of
> this one.
> * *Element type.* {{Instant}} matches crossbeam and is more useful than
> {{null}} or a token; Go sends the
> time too.
> h2. Provenance
> Found while porting Jon Kerridge's _Using Concurrency and Parallelism
> Effectively_ onto
> {{groovy.concurrent}} with a static verifier β the same exercise behind
> GROOVY-12320, GROOVY-12323,
> GROOVY-12324 and GROOVY-12326. Three of the book's chapters put a timer in an
> {{ALT}} (c05's scaling device,
> c14's hand-eye test, c17's sampling {{Sniffer}}), and none of them ports: the
> timeout is part of the choice
> in each, not a wrapper around it. With input, output and boolean guards now
> all present, the timer is the
> one member of JCSP's guard family Groovy's select still lacks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
