kgeisz commented on code in PR #7625: URL: https://github.com/apache/hbase/pull/7625#discussion_r2695183110
########## hbase-it/src/test/java/org/apache/hadoop/hbase/backup/IntegrationTestBackupRestoreBase.java: ########## @@ -90,20 +91,19 @@ public abstract class IntegrationTestBackupRestoreBase extends IntegrationTestBa protected static final int DEFAULT_NUM_ITERATIONS = 10; protected static final int DEFAULT_ROWS_IN_ITERATION = 10000; protected static final String SLEEP_TIME_KEY = "sleeptime"; - // short default interval because tests don't run very long. + // Short default interval because tests don't run very long. protected static final long SLEEP_TIME_DEFAULT = 50000L; protected static String DEFAULT_BACKUP_ROOT_DIR = "backupIT"; - protected static int rowsInIteration; - protected static int regionsCountPerServer; - protected static int regionServerCount; + // These test parameters can be configured using a Configuration object or via the command line. + protected static int rowsInIteration = -1; + protected static int regionsCountPerServer = -1; + protected static int regionServerCount = -1; + protected static int numIterations = -1; + protected static int numTables = -1; + protected static long sleepTime = -1; Review Comment: > 1. why does static require here? It's not required. That's how the test originally had it. I will change it. > 2. is there any other unit test or command line code calling initializeTestParameters ? if we don't set all these variables to -1 and also allow initializeTestParameters to assign value for unit test by using configuration object, does it work ? Currently, only `IntegrationTestContinuousBackupRestore` and `IntegrationTestBackupRestore` use this `initializeTestParameters()` method. I will explain my reasons for why these variable are set to `-1`, and maybe there is a better way to do this. Let's pretend I don't have the `if (variable == -1)` blocks in `initializeTestParameters()` and I am not initializing these variables to be `-1`, and instead I'm just doing `variable = conf.getInt(...)`. If we run the test in IntelliJ or using Maven, then the variable are set up fine. However, when we run in the command line with `bin/hbase` and use custom parameter values, then `IntegrationTestBackupRestoreBase.processOptions()`is executed first, which sets up the variables. After, any setting of variables, such as using `conf` object in `initializeTestParameters()` will overwrite what was supplied in the command line and configured in `processOptions()`. Basically, my idea for initializing the variable to `-1` and using `if (variable == -1)` was a way of saying "this variable has not been set yet, so it's okay to set it here in `initializeTestParameters()`. However, if the variable is no longer `-1`, then that means it shouldn't be set again in `initializeTestParameters()`. -- 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]
