Copilot commented on code in PR #7989: URL: https://github.com/apache/texera/pull/7989#discussion_r3866665395
########## common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/RandomUserAgentSpec.scala: ########## @@ -0,0 +1,169 @@ +/* + * 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.amber.operator.source.fetcher + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.jdk.CollectionConverters._ + +/** + * `RandomUserAgent` picks a browser by weighted draw over a private static frequency + * table and then picks one of that browser's user-agent strings at random. + * + * The weights are hard-coded and sum to 98.6, not 100, while the draw is over + * `[0, 100)`. Roughly 1.4% of calls therefore exhaust the loop without any bucket + * claiming them, leaving `browser` null and taking the `"Chrome"` fallback. Waiting + * for that tail to show up on its own would make the suite flaky and slow, so the + * tests below swap the private static `freqMap` for a table they control and restore + * the original in a `finally`. That is a process-wide mutation, which is safe here + * only because this module runs its suites strictly serially + * (`Global / concurrentRestrictions += Tags.limit(Tags.Test, 1)` in build.sbt) and + * because the swap never outlives a single test. + * + * Deliberately NOT covered: the class's implicit default constructor. `RandomUserAgent` + * is a static-only utility that nothing instantiates, so `new RandomUserAgent()` would + * assert nothing about its behaviour. + */ +class RandomUserAgentSpec extends AnyFlatSpec with Matchers { + + private def declaredField(name: String): java.lang.reflect.Field = { + val field = classOf[RandomUserAgent].getDeclaredField(name) + field.setAccessible(true) + field + } + + private def uaMap: java.util.Map[String, Array[String]] = + declaredField("uaMap").get(null).asInstanceOf[java.util.Map[String, Array[String]]] + + private def freqMap: java.util.Map[String, java.lang.Double] = + declaredField("freqMap").get(null).asInstanceOf[java.util.Map[String, java.lang.Double]] + + /** + * Runs `body` with the private static frequency table replaced by `weights`, then + * restores the original table. Restoring matters: `URLFetchUtil` draws from the same + * static table on every fetch, so a leaked replacement would starve every later suite + * in this JVM. + */ + private def withFreqTable[T](weights: (String, Double)*)(body: => T): T = { + val field = declaredField("freqMap") + val original = field.get(null) + val replacement = new java.util.HashMap[String, java.lang.Double]() + weights.foreach { + case (browser, weight) => + replacement.put(browser, java.lang.Double.valueOf(weight)) + } + field.set(null, replacement) + try body + finally field.set(null, original) + } + + "RandomUserAgent.getRandomUserAgent" should + "fall back to a Chrome agent when no frequency bucket claims the draw" in { + // `Math.random() * 100` lands in [0, 100) and the only bucket contributes -1.0, so + // `rand <= count` is false for every draw: the loop runs to exhaustion, `browser` + // stays null, and the fallback has to supply a browser that actually has agents. + // This is the real ~1.4% tail, not a synthetic state. Review Comment: This comment is a bit misleading: the test forces the fallback path deterministically by replacing the frequency table with a negative weight. It exercises the same code path as the real ~1.4% tail, but the state is still synthetic (the production tail comes from weights summing to 98.6 < 100). -- 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]
