Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow valid agent types in PCT context #660

Merged
merged 2 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,22 @@ public abstract class AbstractModelDefTest extends AbstractDeclarativeTest {
@Rule public GitSampleRepoRule otherRepo = new GitSampleRepoRule();
@Rule public GitSampleRepoRule thirdRepo = new GitSampleRepoRule();

protected static String legalAgentTypes = "";
protected String legalAgentTypes = "";

@BeforeClass
public static void setUpPreClass() throws Exception {
List<String> agentTypes = new ArrayList<>();
@Before
public void setUpPreClass() {
// Ensure that the agent types expected to be contributed by this plugin are present
NavigableSet<String> agentTypes = new TreeSet<>(List.of("any", "label", "none", "otherField"));

// Allow agent types to be contributed by other plugins; for example, the Kubernetes plugin PCT context
for (DeclarativeAgentDescriptor d : j.jenkins.getExtensionList(DeclarativeAgentDescriptor.class)) {
String symbol = symbolFromDescriptor(d);
if (symbol != null) {
agentTypes.add(symbol);
}
}
legalAgentTypes = "[" + StringUtils.join(agentTypes.stream().sorted().collect(Collectors.toList()), ", ") + "]";

legalAgentTypes = "[" + String.join(", ", agentTypes) + "]";
}

private static String symbolFromDescriptor(Descriptor d) {
Expand Down Expand Up @@ -223,7 +226,7 @@ public static Iterable<Object[]> configsWithErrors() {
result.add(new Object[]{"invalidWrapperType", "Invalid option type \"echo\". Valid option types:"});
result.add(new Object[]{"invalidStageWrapperType", "Invalid option type \"echo\". Valid option types:"});

result.add(new Object[]{"unknownBareAgentType", Messages.ModelValidatorImpl_InvalidAgentType("foo", legalAgentTypes)});
result.add(new Object[]{"unknownBareAgentType", Messages.ModelValidatorImpl_InvalidAgentType("foo", "[any, label, none, otherField]")});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newly consistent with unknownAgentType in the sense that both now hardcode a list of agent types rather than using legalAgentTypes, which is not yet initialized when this code is running.

result.add(new Object[]{"agentMissingRequiredParam", Messages.ModelValidatorImpl_MultipleAgentParameters("otherField", "[label, otherField]")});
result.add(new Object[]{"agentUnknownParamForType", Messages.ModelValidatorImpl_InvalidAgentParameter("fruit", "otherField", "[label, otherField, nested]")});
result.add(new Object[]{"notificationsSectionRemoved", "object instance has properties which are not allowed by the schema"});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.htmlunit.util.NameValuePair;
import net.sf.json.JSONObject;
import org.jenkinsci.plugins.pipeline.modeldefinition.AbstractModelDefTest;
import org.jenkinsci.plugins.pipeline.modeldefinition.Messages;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
Expand Down Expand Up @@ -74,6 +75,17 @@ public void testFailedValidateJson() throws Exception {
assertNotNull(resultData);
assertEquals("Result wasn't a failure - " + result.toString(2), "failure", resultData.getString("result"));

/*
* If the error message contains the list of legal agent types, ensure that agent types contributed by other
* plugins (for example, the Kubernetes plugin in PCT context) are present. Note that we can't do this from
* AbstractModelDefTest#configsWithErrors because determining this list requires Jenkins to be started, and
* Jenkins has not yet been started when we are determining the parameters for the JUnit parameterized test.
*/
if (expectedError.equals(
Messages.ModelValidatorImpl_InvalidAgentType("foo", "[any, label, none, otherField]"))) {
expectedError = Messages.ModelValidatorImpl_InvalidAgentType("foo", legalAgentTypes);
}

assertTrue("Errors array (" + resultData.getJSONArray("errors").toString(2) + ") didn't contain expected error '" + expectedError + "'",
foundExpectedErrorInJSON(resultData.getJSONArray("errors"), expectedError));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import org.jenkinsci.plugins.pipeline.modeldefinition.AbstractModelDefTest;
import org.jenkinsci.plugins.pipeline.modeldefinition.BaseParserLoaderTest;
import org.jenkinsci.plugins.pipeline.modeldefinition.Messages;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
Expand All @@ -46,6 +47,17 @@ public static Iterable<Object[]> generateParameters() {

@Test
public void parseAndValidateJSONWithError() throws Exception {
/*
* If the error message contains the list of legal agent types, ensure that agent types contributed by other
* plugins (for example, the Kubernetes plugin in PCT context) are present. Note that we can't do this from
* AbstractModelDefTest#configsWithErrors because determining this list requires Jenkins to be started, and
* Jenkins has not yet been started when we are determining the parameters for the JUnit parameterized test.
*/
if (expectedError.equals(
Messages.ModelValidatorImpl_InvalidAgentType("foo", "[any, label, none, otherField]"))) {
expectedError = Messages.ModelValidatorImpl_InvalidAgentType("foo", legalAgentTypes);
}

findErrorInJSON(expectedError, configName);
}
}
Loading