Skip to content

Commit 0a5b7dd

Browse files
committed
code review fixes
Get windows box IDs from environment variables in static context instead of on plugin application Clarify how the plugin determines the environment variables to run vagrant with
1 parent d32c16a commit 0a5b7dd

File tree

1 file changed

+22
-26
lines changed

1 file changed

+22
-26
lines changed

buildSrc/src/main/groovy/org/elasticsearch/gradle/vagrant/VagrantTestPlugin.groovy

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class VagrantTestPlugin implements Plugin<Project> {
3232
]
3333

3434
/** Windows boxes that are available - map of box names to image IDs **/
35-
static Map<String, String> WINDOWS_BOXES = [:]
35+
static List<String> WINDOWS_BOXES = []
3636

3737
/** All available boxes **/
3838
static List<String> BOXES
@@ -44,7 +44,7 @@ class VagrantTestPlugin implements Plugin<Project> {
4444
]
4545

4646
/** extra env vars to pass to vagrant for box configuration **/
47-
static Map<String, String> VAGRANT_ENV_VARS = [:]
47+
static Map<String, String> VAGRANT_BOX_ENV_VARS = [:]
4848

4949
/** All distributions to bring into test VM, whether or not they are used **/
5050
static List<String> DISTRIBUTIONS = [
@@ -65,11 +65,13 @@ class VagrantTestPlugin implements Plugin<Project> {
6565
private static final String BATS_TEST_COMMAND ="cd \$PACKAGING_ARCHIVES && sudo bats --tap \$BATS_TESTS/*.$BATS"
6666
private static final String PLATFORM_TEST_COMMAND ="rm -rf ~/elasticsearch && rsync -r /elasticsearch/ ~/elasticsearch && cd ~/elasticsearch && ./gradlew test integTest"
6767

68+
static {
69+
collectAvailableBoxes()
70+
}
71+
6872
@Override
6973
void apply(Project project) {
7074

71-
collectAvailableBoxes(project)
72-
7375
// Creates the Vagrant extension for the project
7476
project.extensions.create('esvagrant', VagrantPropertiesExtension, listSelectedBoxes(project))
7577

@@ -100,20 +102,20 @@ class VagrantTestPlugin implements Plugin<Project> {
100102
/**
101103
* Enumerate all the boxes that we know about and could possibly choose to test
102104
*/
103-
private static void collectAvailableBoxes(Project project) {
105+
private static void collectAvailableBoxes() {
104106
String windows_2012r2_box = System.getenv('VAGRANT_WINDOWS_2012R2_BOX')
105107
if (windows_2012r2_box != null && windows_2012r2_box.isEmpty() == false) {
106-
WINDOWS_BOXES['windows-2012r2'] = windows_2012r2_box
107-
VAGRANT_ENV_VARS['VAGRANT_WINDOWS_2012R2_BOX'] = windows_2012r2_box
108+
WINDOWS_BOXES.add('windows-2012r2')
109+
VAGRANT_BOX_ENV_VARS['VAGRANT_WINDOWS_2012R2_BOX'] = windows_2012r2_box
108110
}
109111

110112
String windows_2016_box = System.getenv('VAGRANT_WINDOWS_2016_BOX')
111113
if (windows_2016_box != null && windows_2016_box.isEmpty() == false) {
112-
WINDOWS_BOXES['windows-2016'] = windows_2016_box
113-
VAGRANT_ENV_VARS['VAGRANT_WINDOWS_2016_BOX'] = windows_2016_box
114+
WINDOWS_BOXES.add('windows-2016')
115+
VAGRANT_BOX_ENV_VARS['VAGRANT_WINDOWS_2016_BOX'] = windows_2016_box
114116
}
115117

116-
BOXES = LINUX_BOXES + WINDOWS_BOXES.keySet()
118+
BOXES = LINUX_BOXES + WINDOWS_BOXES
117119
}
118120

119121
/**
@@ -127,7 +129,7 @@ class VagrantTestPlugin implements Plugin<Project> {
127129
case 'linux-all':
128130
return LINUX_BOXES
129131
case 'windows-all':
130-
return WINDOWS_BOXES.keySet().toList()
132+
return WINDOWS_BOXES
131133
case 'all':
132134
return BOXES
133135
default:
@@ -341,20 +343,6 @@ class VagrantTestPlugin implements Plugin<Project> {
341343
createPlatformTestTask(project)
342344
}
343345

344-
private static Map<String, String> vagrantEnvVars(Project project) {
345-
/*
346-
* We always use the main project.rootDir as Vagrant's current working directory (VAGRANT_CWD)
347-
* so that boxes are not duplicated for every Gradle project that use this VagrantTestPlugin.
348-
*/
349-
def vagrantEnvVars = [
350-
'VAGRANT_CWD' : "${project.rootDir.absolutePath}",
351-
'VAGRANT_VAGRANTFILE': 'Vagrantfile',
352-
'VAGRANT_PROJECT_DIR': "${project.projectDir.absolutePath}"
353-
]
354-
vagrantEnvVars.putAll(VAGRANT_ENV_VARS)
355-
return vagrantEnvVars
356-
}
357-
358346
private static void createVagrantBoxesTasks(Project project) {
359347
assert project.extensions.esvagrant.boxes != null
360348

@@ -379,7 +367,15 @@ class VagrantTestPlugin implements Plugin<Project> {
379367
assert project.tasks.platformTest != null
380368
Task platformTest = project.tasks.platformTest
381369

382-
def vagrantEnvVars = vagrantEnvVars(project)
370+
/*
371+
* We always use the main project.rootDir as Vagrant's current working directory (VAGRANT_CWD)
372+
* so that boxes are not duplicated for every Gradle project that use this VagrantTestPlugin.
373+
*/
374+
def vagrantEnvVars = [
375+
'VAGRANT_CWD' : "${project.rootDir.absolutePath}",
376+
'VAGRANT_VAGRANTFILE' : 'Vagrantfile',
377+
'VAGRANT_PROJECT_DIR' : "${project.projectDir.absolutePath}"
378+
] + VAGRANT_BOX_ENV_VARS
383379

384380
// Each box gets it own set of tasks
385381
for (String box : BOXES) {

0 commit comments

Comments
 (0)