Copilot commented on code in PR #6879:
URL: https://github.com/apache/texera/pull/6879#discussion_r3649190775
##########
common/auth/src/test/scala/org/apache/texera/auth/UserActivityTrackerSpec.scala:
##########
@@ -168,4 +173,202 @@ class UserActivityTrackerSpec extends AnyFlatSpec with
Matchers {
noException should be thrownBy tracker.evictStale()
}
+
+ it should "not create a cooldown entry for a null uid" in {
+ val recorder = new Recorder
+ val clock = new
AtomicReference[Instant](Instant.parse("2026-01-01T00:00:00Z"))
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(null)
+
+ tracker.cooldownSize shouldBe 0
+ }
+
+ it should "keep exactly one cooldown entry per uid across repeated calls" in
{
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(42)
+ clock.set(t0.plus(Duration.ofMinutes(1)))
+ tracker.markActive(42)
+ clock.set(t0.plus(Duration.ofMinutes(6)))
+ tracker.markActive(42)
+
+ recorder.calls.size shouldBe 2
+ tracker.cooldownSize shouldBe 1
+ }
+
+ it should "stamp each upsert with the clock value at claim time" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val t1 = t0.plus(Duration.ofMinutes(7))
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(42)
+ // a suppressed call in between must not change the recorded timestamps
+ clock.set(t0.plus(Duration.ofMinutes(3)))
+ tracker.markActive(42)
+ clock.set(t1)
+ tracker.markActive(42)
+
+ recorder.calls.poll() shouldBe ((42, t0))
+ recorder.calls.poll() shouldBe ((42, t1))
+ recorder.calls.size shouldBe 0
+ }
+
+ it should "evict only the stale entries and retain the fresh ones" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(1)
+ clock.set(t0.plus(Duration.ofMinutes(8)))
+ tracker.markActive(2)
+ tracker.cooldownSize shouldBe 2
+
+ // cutoff = t0 + 11min - 10min = t0 + 1min: uid 1 (t0) is stale, uid 2
(t0+8min) is not
+ clock.set(t0.plus(Duration.ofMinutes(11)))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 1
+
+ // uid 2 is still in cooldown, so it must not produce a second write
+ tracker.markActive(2)
+ recorder.calls.size shouldBe 2
+ }
+
+ it should "retain an entry that is exactly 2 * writeInterval old" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(1)
+
+ // cutoff == t0 exactly; the boundary entry is not "before" the cutoff
+ clock.set(t0.plus(Duration.ofMinutes(10)))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 1
+
+ // one millisecond past the boundary it is evicted
+ clock.set(t0.plus(Duration.ofMinutes(10)).plusMillis(1))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 0
+ }
+
+ it should "tolerate evictStale on an empty tracker" in {
+ val recorder = new Recorder
+ val clock = new
AtomicReference[Instant](Instant.parse("2026-01-01T00:00:00Z"))
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ noException should be thrownBy tracker.evictStale()
+
+ tracker.cooldownSize shouldBe 0
+ recorder.calls.size shouldBe 0
+ }
+
+ it should "re-claim a uid whose entry was evicted" in {
+ val recorder = new Recorder
+ val t0 = Instant.parse("2026-01-01T00:00:00Z")
+ val clock = new AtomicReference[Instant](t0)
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ tracker.markActive(1)
+ clock.set(t0.plus(Duration.ofMinutes(11)))
+ tracker.evictStale()
+ tracker.cooldownSize shouldBe 0
+
+ tracker.markActive(1)
+
+ recorder.calls.size shouldBe 2
+ tracker.cooldownSize shouldBe 1
+ }
+
+ it should "perform exactly one upsert when many threads race on the same
uid" in {
+ val recorder = new Recorder
+ val clock = new
AtomicReference[Instant](Instant.parse("2026-01-01T00:00:00Z"))
+ val tracker = makeTracker(Duration.ofMinutes(5), recorder, clock)
+
+ val start = new CountDownLatch(1)
+ val threads = (1 to 16).map(_ =>
+ new Thread(() => {
+ start.await()
+ tracker.markActive(99)
+ })
+ )
+ threads.foreach(_.start())
+ start.countDown()
+ threads.foreach(_.join())
+
Review Comment:
The concurrency test uses `Thread.join()` with no timeout, which can hang
the entire test suite indefinitely if any thread fails to terminate (e.g., due
to a regression in `markActive`). Adding a bounded join and asserting all
threads completed keeps CI failures fast and diagnosable.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
