User inputs to get.sh are no longer overwritten, even when USE_TESTENV_PROPERTIES=true

[jmjaffe37]

Currently, if a user sets USE_TESTENV_PROPERTIES=true, then inputs like "--tkg_repo" and "--tkg_branch" are ignored. This PR ensures that they are no-longer ignored.

[smlambert]

USE_TESTENV_PROPERTIES=true was never meant to be overwritten, it was meant to guarantee that the values in the testenv.properties file were the ones used.

[karianna]

USE_TESTENV_PROPERTIES=true was never meant to be overwritten, it was meant to guarantee that the values in the testenv.properties file were the ones used.

Should this be closed in that case?

[smlambert]

Should this be closed in that case?

Before closing, I guess we should ensure that whatever the use case that @jmjaffe37 is trying to address is accomplished by USE_TESTENV_PROPERTIES=false (or not setting it, as the default value is false), and then overwriting any of the test repositories parameters (as listed in this documentation: https://github.com/adoptium/aqa-tests/blob/master/docs/pages/JenkinFeatures.md#test-repositories-parameters).

[jmjaffe37]

@ShelleyLambert @karianna, here is the use case that I am envisioning: the user wants to use all of the testenv.properties inputs except for the ones specified by the flags. This was my use-case recently because I was testing out running AQA without ant-contrib, so I needed to set the TKG branch to master instead of the latest tag (only master had the ant-contrib removal merged in)

[smlambert]

re: #7129 (comment) - in that case you could edit the values in the testenv.properties file to point at the TKG repo and branch with those changes.

[jmjaffe37]

Is that preferred? As a user, it took me a while to realize how/why my script inputs via flags were being ignored. I would imagine that other users would have the same experience.

Another potential idea: what if I introduce the env var USE_OVERWRITEABLE_TESTENV_PROPERTIES? This way, we can have the behavior that I am expecting, and the people who want their inputs to be ignored can continue having them ignored.

This would be quick to implement since we just change the if-statement conditional to source testenv.properties if either USE_TESTENV_PROPERTIES == true or USE_OVERWRITEABLE_TESTENV_PROPERTIES== true. Then, we only run the function replayTestenvOverrides if USE_OVERWRITEABLE_TESTENV_PROPERTIES== true.

[smlambert]

Will have to think about it. Quick to implement is not the issue. Its the philosophy of the reason we even pin to that file that I hesitate to lose.

Currently if USE_TESTENV_PROPERTIES=true, we keep the testenv.properties file as one of the artifacts that is saved and used in a 'rerun' with same inputs. When it is false, we capture and create a new testenv.properties files with all of the test repo parameters. We do not currently consider a middle path, and very much still need a way to enforce strict use of testenv.properties file for anyone wishing to list their JDK distribution as "AQAvit verified".

But I do recognize that this would be a feature that supports development work.