dsmiley commented on code in PR #2062:
URL: https://github.com/apache/solr/pull/2062#discussion_r1911881108


##########
gradle/testing/randomization.gradle:
##########
@@ -199,7 +199,12 @@ allprojects {
 
         // Enable security manager, if requested. We could move the selection 
of security manager and security policy
         // to each project's build/ configuration but it seems compact enough 
to keep it here for now.
-        if 
(Boolean.parseBoolean(testOptionsResolved["tests.useSecurityManager"])) {
+        def useSecurityManager = 
Boolean.parseBoolean(testOptionsResolved["tests.useSecurityManager"]);

Review Comment:
   I glossed over testOptionsResolved but glad you pointed this out now.  This 
is a better example than those I listed.  Basically, my point is why don't you 
go enhance randomization.gradle to compute testOptionsResolved with a new 
ability for a gradle module/project change tweak *whatever* test options 
instead of making useSecurityManager a special case.  I'm actually surprised, 
given all the flexibility here, that it's not _already_ possible.  I think it 
is actually... notice it calls propertyOrDefault which in turn consults project 
properties.



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to