Skip to content

Commit

Permalink
fix(config/test): provide a non-null halconfigDirectory in HalconfigP…
Browse files Browse the repository at this point in the history
…arserMocker (#2172)

* test(config): validate spring config behavior with HalconfigDirectoryStructure

to pave the way for some refactoring to support java 17

* refactor(config): use constructor injection in HalconfigParser

as a general improvement and also to pave the way to cleanly specify a non-null
halconfigDirectory during testing.

* refactor(config): use constructor injection in HalconfigDirectoryStructure

as a general improvement and also to pave the way to cleanly specify a non-null
halconfigDirectory during testing.

* fix(config/test): provide a non-null halconfigDirectory in HalconfigParserMocker

since tests like this fail in java 17 otherwise:

> Task :halyard-config:test

AccountServiceSpec > load an existent account node FAILED
    java.lang.NullPointerException
        at java.base/java.util.Objects.requireNonNull(Objects.java:209)
        at java.base/sun.nio.fs.UnixFileSystem.getPath(UnixFileSystem.java:263)
        at java.base/java.nio.file.Path.of(Path.java:147)
        at java.base/java.nio.file.Paths.get(Paths.java:69)
        at com.netflix.spinnaker.halyard.config.config.v1.HalconfigDirectoryStructure.getHalconfigPath(HalconfigDirectoryStructure.java:50)
        at com.netflix.spinnaker.halyard.config.config.v1.HalconfigParser.transformHalconfig(HalconfigParser.java:159)
        at com.netflix.spinnaker.halyard.config.services.v1.HalconfigParserMocker.mockHalconfigParser(HalconfigParserMocker.groovy:38)
        at com.netflix.spinnaker.halyard.config.services.v1.AccountServiceSpec.makeAccountService(AccountServiceSpec.groovy:34)
        at com.netflix.spinnaker.halyard.config.services.v1.AccountServiceSpec.load an existent account node(AccountServiceSpec.groovy:56)

* refactor(config): use RequiredArgsConstructor in HalconfigDirectoryStructure

to reduce boilerplate
  • Loading branch information
dbyron-sf authored Jul 29, 2024
1 parent 532fff3 commit 5812e7b
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 13 deletions.
1 change: 1 addition & 0 deletions halyard-config/halyard-config.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ dependencies {
testImplementation 'org.junit.jupiter:junit-jupiter-api'
testImplementation 'org.spockframework:spock-core'
testImplementation 'org.springframework:spring-test'
testImplementation 'org.springframework.boot:spring-boot-starter-test'
testImplementation 'org.codehaus.groovy:groovy'
testRuntimeOnly 'net.bytebuddy:byte-buddy'
testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,19 @@
import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
import lombok.Setter;
import org.springframework.beans.factory.annotation.Autowired;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;

@Component
@RequiredArgsConstructor
public class HalconfigDirectoryStructure {
private static ThreadLocal<String> directoryOverride = new ThreadLocal<>();

public static void setDirectoryOverride(String directory) {
directoryOverride.set(directory);
}

@Autowired @Setter String halconfigDirectory;
private final String halconfigDirectory;

public String getHalconfigDirectory() {
String directory = directoryOverride.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,24 @@
@Slf4j
@Component
public class HalconfigParser {
@Autowired StrictObjectMapper objectMapper;
private final StrictObjectMapper objectMapper;

@Autowired HalconfigDirectoryStructure halconfigDirectoryStructure;
private final HalconfigDirectoryStructure halconfigDirectoryStructure;

@Autowired ApplicationContext applicationContext;
private final ApplicationContext applicationContext;

private boolean useBackup = false;

@Autowired
public HalconfigParser(
StrictObjectMapper objectMapper,
HalconfigDirectoryStructure halconfigDirectoryStructure,
ApplicationContext applicationContext) {
this.objectMapper = objectMapper;
this.halconfigDirectoryStructure = halconfigDirectoryStructure;
this.applicationContext = applicationContext;
}

private Yaml getYamlParser() {
return applicationContext.getBean(Yaml.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ class HalconfigParserSpec extends Specification {
void setup() {
ApplicationContext applicationContext = Stub(ApplicationContext.class)
applicationContext.getBean(Yaml.class) >> new Yaml(new SafeConstructor())
parser = new HalconfigParser()
parser.applicationContext = applicationContext
parser.objectMapper = new StrictObjectMapper()
parser = new HalconfigParser(new StrictObjectMapper(), null /* halconfigDirectoyStructure */, applicationContext)
}

void "Accept minimal config"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,18 @@ import java.nio.charset.StandardCharsets

class HalconfigParserMocker extends Specification {
HalconfigParser mockHalconfigParser(String config) {
def parserStub = new HalconfigParser()
ApplicationContext applicationContext = Stub(ApplicationContext.class)
applicationContext.getBean(Yaml.class) >> new Yaml(new SafeConstructor())
parserStub.objectMapper = new StrictObjectMapper()
parserStub.applicationContext = applicationContext
parserStub.halconfigDirectoryStructure = new HalconfigDirectoryStructure();

// HalconfigDirectoryStructure needs a non-null halconfigDirectory for
// Paths.get(getHalconfigDirectory()) to work properly. The directory
// doesn't actually need to exist though. So, instead of
//
// String halconfigDirectory = Files.createTempDirectory("halyard-test")
//
// let's use an arbitrary string
String halconfigDirectory = "halyard-test"
def parserStub = new HalconfigParser(new StrictObjectMapper(), new HalconfigDirectoryStructure(halconfigDirectory), applicationContext)

def stream = new ByteArrayInputStream(config.getBytes(StandardCharsets.UTF_8))
Halconfig halconfig = parserStub.parseHalconfig(stream)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright 2024 Salesforce, Inc.
*
* Licensed 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 com.netflix.spinnaker.halyard.config.config.v1;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.springframework.boot.context.annotation.UserConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;

class HalconfigConfigurationTest {

private final ApplicationContextRunner runner =
new ApplicationContextRunner()
.withConfiguration(
UserConfigurations.of(ResourceConfig.class, HalconfigDirectoryStructure.class));

@BeforeEach
void init(TestInfo testInfo) {
System.out.println("--------------- Test " + testInfo.getDisplayName());
}

@Test
void testHalconfigDirectory() {
runner.run(
ctx -> {
String homeDir = System.getProperty("user.home");
assertThat(homeDir).isNotNull();

assertThat(ctx).hasSingleBean(HalconfigDirectoryStructure.class);
String halconfigDirectory = ctx.getBean("halconfigDirectory", String.class);
assertThat(halconfigDirectory).isNotNull();
assertThat(halconfigDirectory).isEqualTo(homeDir + "/.hal");
HalconfigDirectoryStructure halconfigDirectoyStructure =
ctx.getBean(HalconfigDirectoryStructure.class);
assertThat(halconfigDirectoyStructure.getHalconfigDirectory())
.isEqualTo(halconfigDirectory);
});
}
}

0 comments on commit 5812e7b

Please sign in to comment.