Skip to content

Commit

Permalink
fix(aws/test): Flaky test update (spinnaker#5621)
Browse files Browse the repository at this point in the history
This test was failing whenever the suffix is generated before the second
hand turns and the expected string is calculated for a different
timestamp. Now the test uses a fixed clock using the instant the test
starts.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
2 people authored and nhtzr committed Aug 18, 2022
1 parent 67b58a7 commit c6ea344
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,23 @@
import com.netflix.spinnaker.clouddriver.aws.services.SecurityGroupService;
import com.netflix.spinnaker.clouddriver.helpers.OperationPoller;
import com.netflix.spinnaker.config.AwsConfiguration.DeployDefaults;
import com.netflix.spinnaker.kork.annotations.VisibleForTesting;
import java.time.Clock;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.temporal.ChronoField;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.joda.time.LocalDateTime;

/**
* A helper class for utility methods related to {@link AutoScalingWorker.AsgConfiguration} and
Expand Down Expand Up @@ -298,8 +304,27 @@ private static String createSecurityGroupForApp(
return applicationSecurityGroupId;
}

private static String createDefaultSuffix() {
return new LocalDateTime().toString("MMddYYYYHHmmss");
private static final AtomicReference<Clock> CLOCK_REF =
new AtomicReference<>(Clock.systemDefaultZone());

@VisibleForTesting
static void setClock(Clock clock) {
CLOCK_REF.setOpaque(clock);
}

private static final DateTimeFormatter SUFFIX_DATE_FORMATTER =
new DateTimeFormatterBuilder()
.appendValue(ChronoField.MONTH_OF_YEAR, 2)
.appendValue(ChronoField.DAY_OF_MONTH, 2)
.appendValue(ChronoField.YEAR)
.appendValue(ChronoField.HOUR_OF_DAY, 2)
.appendValue(ChronoField.MINUTE_OF_HOUR, 2)
.appendValue(ChronoField.SECOND_OF_MINUTE, 2)
.toFormatter();

@VisibleForTesting
static String createDefaultSuffix() {
return LocalDateTime.now(CLOCK_REF.getOpaque()).format(SUFFIX_DATE_FORMATTER);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,29 @@ import com.netflix.spinnaker.clouddriver.aws.deploy.description.BasicAmazonDeplo
import com.netflix.spinnaker.clouddriver.aws.model.AmazonBlockDevice
import com.netflix.spinnaker.config.AwsConfiguration
import com.netflix.spinnaker.clouddriver.aws.services.SecurityGroupService
import org.joda.time.LocalDateTime
import spock.lang.Specification
import spock.lang.Unroll

import java.time.Clock
import java.time.Instant
import java.time.ZoneId

class AsgConfigHelperSpec extends Specification {
def securityGroupServiceMock = Mock(SecurityGroupService)
def deployDefaults = new AwsConfiguration.DeployDefaults()
def asgConfig = new AutoScalingWorker.AsgConfiguration(
application: "fooTest",
stack: "stack")

void setup() {
// test code shouldn't assume it will run in less than one second, so let's control the clock
AsgConfigHelper.clock = Clock.fixed(Instant.now(), ZoneId.systemDefault())
}

void cleanup() {
AsgConfigHelper.clock = Clock.systemDefaultZone()
}

void "should return name correctly"() {
when:
def actualName = AsgConfigHelper.createName(baseName, suffix)
Expand All @@ -47,8 +59,8 @@ class AsgConfigHelperSpec extends Specification {
where:
baseName | suffix || expectedName
"base" | "suffix" || "base-suffix"
"base" | null || "base-${new LocalDateTime().toString("MMddYYYYHHmm")}"
"base" | "" || "base-${new LocalDateTime().toString("MMddYYYYHHmm")}"
"base" | null || "base-${AsgConfigHelper.createDefaultSuffix()}"
"base" | "" || "base-${AsgConfigHelper.createDefaultSuffix()}"
}

void "should lookup security groups when provided by name"() {
Expand Down

0 comments on commit c6ea344

Please sign in to comment.