mengw15 commented on code in PR #7502: URL: https://github.com/apache/texera/pull/7502#discussion_r3746769513
########## common/auth/src/test/scala/org/apache/texera/auth/UserActivityTrackerDbSpec.scala: ########## @@ -0,0 +1,129 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.auth + +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.Tables.{USER, USER_LAST_ACTIVE_TIME} +import org.apache.texera.dao.jooq.generated.tables.daos.UserDao +import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.{OffsetDateTime, ZoneOffset} + +/** Exercises the production singleton's *default* upsert against a real + * database. [[UserActivityTrackerSpec]] covers the cooldown / CAS / eviction + * logic with an injected `upsertFn`, so this suite only has to reach the one + * path that spec cannot: the jOOQ write itself. + * + * Kept in its own file so the pure-logic spec stays free of embedded Postgres. + */ +class UserActivityTrackerDbSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + // The singleton's per-uid cooldown lives for the life of the JVM, so every + // test claims a uid of its own rather than sharing one. + private val writeUid: Integer = 90001 + private val cooldownUid: Integer = 90002 + private val barrierUid: Integer = 90003 Review Comment: Adopted — uids are now drawn per run (`90000 + Random.nextInt(900000)`, one per test), matching what `UserQuotaResourceSpec` already does. For the record, I tried to reproduce the failure first and could not: `sbt "Auth/testOnly …DbSpec" "Auth/testOnly …DbSpec"` in one session passes both times. `Auth` doesn't set `Test / fork`, but sbt hands each test run a fresh classloader, so the Scala `object` — and with it the cooldown map — is re-initialized. Still worth changing: correctness here shouldn't depend on sbt's classloader policy, and the fix is two lines. ########## common/auth/src/test/scala/org/apache/texera/auth/UserActivityTrackerDbSpec.scala: ########## @@ -0,0 +1,129 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.auth + +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.Tables.{USER, USER_LAST_ACTIVE_TIME} +import org.apache.texera.dao.jooq.generated.tables.daos.UserDao +import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.{OffsetDateTime, ZoneOffset} + +/** Exercises the production singleton's *default* upsert against a real + * database. [[UserActivityTrackerSpec]] covers the cooldown / CAS / eviction + * logic with an injected `upsertFn`, so this suite only has to reach the one + * path that spec cannot: the jOOQ write itself. + * + * Kept in its own file so the pure-logic spec stays free of embedded Postgres. + */ Review Comment: Agreed, and changed to `Part of #7498`. The `VFSURIFactory` half isn't skipped for convenience — its only uncovered lines are the three guard `throw`s plus an unused default argument, and `createNonResultVFSURI` is `private` with two callers that both pass fixed arguments, so none of them can fire. Covering them needs either `private[storage]` or reflection; I left that out of a test-only PR and noted it in the description so you can decide whether to re-scope the issue or take the visibility change. -- 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]
