Skip to content

Commit

Permalink
fix issue 4469: orca-core skipping most tests in build (#2957)
Browse files Browse the repository at this point in the history
* fix(tests): rename fields in LockContextSpec

works around the java.lang.VerifyError mentioned in
spinnaker/spinnaker#4469

* fix(ExecutionSpec): fix test failure due to slf4j-simple

With LockContextSpec fixed in the previous commit to no longer break the
junit-vintage test runner, ExecutionSpec was also broken in commit
a1de32d due to the change from using log4j's
MDC to slf4j's MDC class.

orca-core was setting `slf4j-simple` as `testRuntimeScope` - and
`slf4j-simple` has a no-op MDC implementation which quietly discards any
values set via `MDC.put()`.

To fix this, allow the MDCAdapter provided by `slf4j-api` to be used
instead by removing the dependency on slf4j-simple, and add an assertion
to the test class to ensure that any MDCAdapter instance besides the
no-op adapter is in use.
  • Loading branch information
mattnworb authored and ezimanyi committed Jun 3, 2019
1 parent 3e66d2a commit fa5b8bc
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 13 deletions.
1 change: 0 additions & 1 deletion orca-core/orca-core.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ dependencies {
testImplementation("org.junit.jupiter:junit-jupiter-api")
testImplementation("org.assertj:assertj-core")

testRuntimeOnly("org.slf4j:slf4j-simple")
testRuntimeOnly("org.junit.jupiter:junit-jupiter-api")
testRuntimeOnly "org.junit.vintage:junit-vintage-engine"
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,54 +10,54 @@ class LockContextSpec extends Specification {

def "builder uses explicitly provided id in when present"() {
given:
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, type, explicitId, stage)
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, lockType, explicitId, stage)

expect:
builder.build() == expected

where:
application = 'app'
type = 'pipeline'
lockType = 'pipeline'
explicitId = 'bacon'

stage = ExecutionBuilder.stage {
context = [:]
}

expectedId = explicitId
expected = new LockManager.LockValue(application, type, expectedId)
expected = new LockManager.LockValue(application, lockType, expectedId)
}

def "builder uses execution id in simple execution"() {
given:
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, type, explicitId, stage)
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, lockType, explicitId, stage)

expect:
builder.build() == expected

where:
application = 'app'
type = 'pipeline'
lockType = 'pipeline'
explicitId = null

stage = ExecutionBuilder.stage {
context = [:]
}

expectedId = stage.execution.id
expected = new LockManager.LockValue(application, type, expectedId)
expected = new LockManager.LockValue(application, lockType, expectedId)
}

def "builder traverses up the hierarchy when execution is triggered by a PipelineTrigger"() {
given:
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, type, explicitId, stage)
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, lockType, explicitId, stage)

expect:
builder.build() == expected

where:
application = 'app'
type = 'pipeline'
lockType = 'pipeline'
explicitId = null

parentStage = ExecutionBuilder.stage {
Expand All @@ -75,20 +75,20 @@ class LockContextSpec extends Specification {
stage = exec.stages[0]

expectedId = parentStage.execution.id
expected = new LockManager.LockValue(application, type, expectedId)
expected = new LockManager.LockValue(application, lockType, expectedId)

}

def "builder traverses up the hierarchy multiple levels when execution is triggered by a PipelineTrigger"() {
given:
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, type, explicitId, stage)
def builder = new LockContext.LockContextBuilder.LockValueBuilder(application, lockType, explicitId, stage)

expect:
builder.build() == expected

where:
application = 'app'
type = 'pipeline'
lockType = 'pipeline'
explicitId = null


Expand All @@ -103,7 +103,7 @@ class LockContextSpec extends Specification {
stage = exec.stages[0]

expectedId = grandParentStage.execution.id
expected = new LockManager.LockValue(application, type, expectedId)
expected = new LockManager.LockValue(application, lockType, expectedId)

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,19 @@ package com.netflix.spinnaker.orca.pipeline.model

import com.netflix.spinnaker.security.AuthenticatedRequest
import org.slf4j.MDC
import org.slf4j.helpers.NOPMDCAdapter
import spock.lang.Specification
import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage

class ExecutionSpec extends Specification {
void setupSpec() {
if (MDC.getMDCAdapter() instanceof NOPMDCAdapter) {
throw new IllegalStateException("ExecutionSpec.AuthenticationDetails tests cannot function " +
"without a real MDC implementation loaded by slf4j")
}
}

def "should return Optional.empty if no authenticated details available"() {
given:
MDC.clear()
Expand Down

0 comments on commit fa5b8bc

Please sign in to comment.