Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-43507] Legacy constructor needs to apply default discovery b…
…ehaviours

- BlueOcean continually destroys and recreates the SCMNavigator without round-tripping through stapler form-binding
  consequently it does not pick up the legacy default discovery traits.
- Also found a trait duplication bug in GitHubSCMSource that was missed due to a faulty test but caught when replicating
  the new legacy constructor tests from navigator into source 'just to be sure to be sure'
  • Loading branch information
stephenc committed Jun 22, 2017
1 parent d4f2fda commit 394d016
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 5 deletions.
Expand Up @@ -227,7 +227,12 @@ public GitHubSCMNavigator(String apiUri, String repoOwner, String scanCredential
this(repoOwner);
setCredentialsId(scanCredentialsId);
setApiUri(apiUri);
if (checkoutCredentialsId != null && !DescriptorImpl.SAME.equals(checkoutCredentialsId)) {
// legacy constructor means legacy defaults
this.traits = new ArrayList<>();
this.traits.add(new BranchDiscoveryTrait(true, true));
this.traits.add(new ForkPullRequestDiscoveryTrait(EnumSet.of(ChangeRequestCheckoutStrategy.HEAD),
new ForkPullRequestDiscoveryTrait.TrustContributors()));
if (!GitHubSCMSource.DescriptorImpl.SAME.equals(checkoutCredentialsId)) {
traits.add(new SSHCheckoutTrait(checkoutCredentialsId));
}
}
Expand Down
Expand Up @@ -311,7 +311,6 @@ public GitHubSCMSource(@CheckForNull String id, @CheckForNull String apiUri, @No
setCredentialsId(scanCredentialsId);
// legacy constructor means legacy defaults
this.traits = new ArrayList<>();
this.traits.add(new SSHCheckoutTrait(checkoutCredentialsId));
this.traits.add(new BranchDiscoveryTrait(true, true));
this.traits.add(new ForkPullRequestDiscoveryTrait(EnumSet.of(ChangeRequestCheckoutStrategy.MERGE), new ForkPullRequestDiscoveryTrait.TrustContributors()));
if (!DescriptorImpl.SAME.equals(checkoutCredentialsId)) {
Expand Down
Expand Up @@ -1508,6 +1508,76 @@ public void build_111111() throws Exception {
);
}

@Test
public void given__legacyCode__when__constructor_cloud__then__discoveryTraitDefaults() throws Exception {
GitHubSCMNavigator instance = new GitHubSCMNavigator(
null,
"cloudbeers",
"bcaef157-f105-407f-b150-df7722eab6c1",
"SAME"
);
assertThat(instance.id(), is("https://api.github.com::cloudbeers"));
assertThat(instance.getRepoOwner(), is("cloudbeers"));
assertThat(instance.getApiUri(), is(nullValue()));
assertThat(instance.getCredentialsId(), is("bcaef157-f105-407f-b150-df7722eab6c1"));
assertThat(instance.getTraits(),
containsInAnyOrder(
Matchers.<SCMTrait<?>>allOf(
instanceOf(BranchDiscoveryTrait.class),
hasProperty("buildBranch", is(true)),
hasProperty("buildBranchesWithPR", is(true))
),
Matchers.<SCMTrait<?>>allOf(
instanceOf(ForkPullRequestDiscoveryTrait.class),
hasProperty("strategyId", is(2)),
hasProperty("trust", instanceOf(ForkPullRequestDiscoveryTrait.TrustContributors.class))
)
)
);
// legacy API
assertThat(instance.getCheckoutCredentialsId(), is("SAME"));
assertThat(instance.getPattern(), is(".*"));
assertThat(instance.getIncludes(), is("*"));
assertThat(instance.getExcludes(), is(""));
}

@Test
public void given__legacyCode__when__constructor_server__then__discoveryTraitDefaults() throws Exception {
GitHubSCMNavigator instance = new GitHubSCMNavigator(
"https://github.test/api/v3",
"cloudbeers",
"bcaef157-f105-407f-b150-df7722eab6c1",
"8b2e4f77-39c5-41a9-b63b-8d367350bfdf"
);
assertThat(instance.id(), is("https://github.test/api/v3::cloudbeers"));
assertThat(instance.getRepoOwner(), is("cloudbeers"));
assertThat(instance.getApiUri(), is("https://github.test/api/v3"));
assertThat(instance.getCredentialsId(), is("bcaef157-f105-407f-b150-df7722eab6c1"));
assertThat(instance.getTraits(),
containsInAnyOrder(
Matchers.<SCMTrait<?>>allOf(
instanceOf(BranchDiscoveryTrait.class),
hasProperty("buildBranch", is(true)),
hasProperty("buildBranchesWithPR", is(true))
),
Matchers.<SCMTrait<?>>allOf(
instanceOf(ForkPullRequestDiscoveryTrait.class),
hasProperty("strategyId", is(2)),
hasProperty("trust", instanceOf(ForkPullRequestDiscoveryTrait.TrustContributors.class))
),
Matchers.<SCMTrait<?>>allOf(
Matchers.instanceOf(SSHCheckoutTrait.class),
hasProperty("credentialsId", is("8b2e4f77-39c5-41a9-b63b-8d367350bfdf"))
)
)
);
// legacy API
assertThat(instance.getCheckoutCredentialsId(), is("8b2e4f77-39c5-41a9-b63b-8d367350bfdf"));
assertThat(instance.getPattern(), is(".*"));
assertThat(instance.getIncludes(), is("*"));
assertThat(instance.getExcludes(), is(""));
}

@Test
public void given__instance__when__setTraits_empty__then__traitsEmpty() {
GitHubSCMNavigator instance = new GitHubSCMNavigator("test");
Expand Down Expand Up @@ -1630,10 +1700,13 @@ public void given__legacyCode__when__checkoutCredentials_SAME__then__noTraitAdde
}

@Test
public void given__legacyCode__when__checkoutCredentials_null__then__noTraitAdded() {
public void given__legacyCode__when__checkoutCredentials_null__then__traitAdded_ANONYMOUS() {
GitHubSCMNavigator instance = new GitHubSCMNavigator(null, "test", "scan", null);
assertThat(instance.getCheckoutCredentialsId(), is(GitHubSCMNavigator.DescriptorImpl.SAME));
assertThat(instance.getTraits(), not(Matchers.<SCMTrait<?>>hasItem(instanceOf(SSHCheckoutTrait.class))));
assertThat(instance.getCheckoutCredentialsId(), is(GitHubSCMSource.DescriptorImpl.ANONYMOUS));
assertThat(instance.getTraits(), Matchers.<SCMTrait<?>>hasItem(allOf(
instanceOf(SSHCheckoutTrait.class),
hasProperty("credentialsId", is(nullValue()))
)));
}

@Test
Expand Down
Expand Up @@ -25,6 +25,7 @@
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;

Expand Down Expand Up @@ -348,6 +349,81 @@ public void use_agent_checkout() throws Exception {
assertThat(instance.getBuildForkPRMerge(), is(true));
}

@Test
public void given__legacyCode__when__constructor_cloud__then__discoveryTraitDefaults() throws Exception {
GitHubSCMSource instance = new GitHubSCMSource("preserve-id", null, "SAME",
"e4d8c11a-0d24-472f-b86b-4b017c160e9a", "cloudbeers", "stunning-adventure");
assertThat(instance.getId(), is("preserve-id"));
assertThat(instance.getApiUri(), is(nullValue()));
assertThat(instance.getRepoOwner(), is("cloudbeers"));
assertThat(instance.getRepository(), is("stunning-adventure"));
assertThat(instance.getCredentialsId(), is("e4d8c11a-0d24-472f-b86b-4b017c160e9a"));
assertThat(instance.getTraits(),
containsInAnyOrder(
Matchers.<SCMSourceTrait>allOf(
instanceOf(BranchDiscoveryTrait.class),
hasProperty("buildBranch", is(true)),
hasProperty("buildBranchesWithPR", is(true))
),
Matchers.<SCMSourceTrait>allOf(
instanceOf(ForkPullRequestDiscoveryTrait.class),
hasProperty("strategyId", is(1)),
hasProperty("trust", instanceOf(ForkPullRequestDiscoveryTrait.TrustContributors.class))
)
)
);
// Legacy API
assertThat(instance.getCheckoutCredentialsId(), is(GitHubSCMSource.DescriptorImpl.SAME));
assertThat(instance.getIncludes(), is("*"));
assertThat(instance.getExcludes(), is(""));
assertThat(instance.getBuildOriginBranch(), is(true));
assertThat(instance.getBuildOriginBranchWithPR(), is(true));
assertThat(instance.getBuildOriginPRHead(), is(false));
assertThat(instance.getBuildOriginPRMerge(), is(false));
assertThat(instance.getBuildForkPRHead(), is(false));
assertThat(instance.getBuildForkPRMerge(), is(true));
}

@Test
public void given__legacyCode__when__constructor_server__then__discoveryTraitDefaults() throws Exception {
GitHubSCMSource instance = new GitHubSCMSource(null, "https://github.test/api/v3",
"8b2e4f77-39c5-41a9-b63b-8d367350bfdf", "e4d8c11a-0d24-472f-b86b-4b017c160e9a",
"cloudbeers", "stunning-adventure");
assertThat(instance.getId(), is(notNullValue()));
assertThat(instance.getApiUri(), is("https://github.test/api/v3"));
assertThat(instance.getRepoOwner(), is("cloudbeers"));
assertThat(instance.getRepository(), is("stunning-adventure"));
assertThat(instance.getCredentialsId(), is("e4d8c11a-0d24-472f-b86b-4b017c160e9a"));
assertThat(instance.getTraits(),
containsInAnyOrder(
Matchers.<SCMSourceTrait>allOf(
instanceOf(BranchDiscoveryTrait.class),
hasProperty("buildBranch", is(true)),
hasProperty("buildBranchesWithPR", is(true))
),
Matchers.<SCMSourceTrait>allOf(
instanceOf(ForkPullRequestDiscoveryTrait.class),
hasProperty("strategyId", is(1)),
hasProperty("trust", instanceOf(ForkPullRequestDiscoveryTrait.TrustContributors.class))
),
Matchers.<SCMSourceTrait>allOf(
Matchers.instanceOf(SSHCheckoutTrait.class),
hasProperty("credentialsId", is("8b2e4f77-39c5-41a9-b63b-8d367350bfdf"))
)
)
);
// Legacy API
assertThat(instance.getCheckoutCredentialsId(), is("8b2e4f77-39c5-41a9-b63b-8d367350bfdf"));
assertThat(instance.getIncludes(), is("*"));
assertThat(instance.getExcludes(), is(""));
assertThat(instance.getBuildOriginBranch(), is(true));
assertThat(instance.getBuildOriginBranchWithPR(), is(true));
assertThat(instance.getBuildOriginPRHead(), is(false));
assertThat(instance.getBuildOriginPRMerge(), is(false));
assertThat(instance.getBuildForkPRHead(), is(false));
assertThat(instance.getBuildForkPRMerge(), is(true));
}

@Test
public void given__instance__when__setTraits_empty__then__traitsEmpty() {
GitHubSCMSource instance = new GitHubSCMSource("testing", "test-repo");
Expand Down

0 comments on commit 394d016

Please sign in to comment.