#917 - Add test-harness module + change json schema test API to return list of errors #1215

timja · 2019-12-03T07:52:01Z

Note: the json schema test API has never been released.

Previously errors in tests just said Assertion failed, with this change a list of exact errors are returned and the error looks like:

assertThat(validateSchema(convertYamlFileToJson(this, "invalidSchemaConfig.yml")),
            empty());

java.lang.AssertionError: 
Expected: an empty collection
     but: <[#/jenkins/numExecutors: expected type: Number, found: String]>

This can then be asserted on to check expected errors, or empty

The test harness module had to be introduced so that the json schema dependency would get included, it seems test-jars don't include dependencies. All tests that depended on classes from the misc test package had to be moved to the test-harness module so that there was no cyclic dependency

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or in Jenkins JIRA
Link to relevant pull requests, esp. upstream and downstream changes
Did you provide a test-case? That demonstrates feature works or fixes the issue.

timja · 2019-12-03T07:53:35Z

I extracted this from #1027 as we may want to do a release for #1214 and the test framework is on master, I would rather not release an API that isn't as good as it could be

jetersen

LGTM

sladyn98 · 2019-12-03T09:13:24Z

plugin/src/test/java/io/jenkins/plugins/casc/misc/Util.java

        try {
            returnSchema().validate(jsonSubject);
+        } catch (ValidationException e) {
+            return e.getAllMessages();
        } catch (Exception ie) {


Is this still required, would reducing them to one block make sense

What do you mean? this is the whole point of the change, the validation messages are now returned as a list.

timja · 2019-12-03T10:15:58Z

Hmm

Caused by: java.lang.ClassNotFoundException: org.everit.json.schema.ValidationException
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
	... 22 more

timja · 2019-12-04T08:18:08Z

Not having a fun time with this, the test jar doesn't want to include the dependency on the json schema dependency.

If I add the dependency to the integrations module that works but not good enough for other consumers who need it

timja · 2019-12-04T16:34:10Z

If anyone has time to look at this it would be much appreciated

jetersen · 2019-12-05T12:16:12Z

I'll have a look after Jenkins world 😅

timja · 2019-12-05T12:17:38Z

I'll have a look after Jenkins world 😅

I tried fiddling with the dependency set but didn't manage to make it work

timja · 2019-12-07T15:33:31Z

https://maven.apache.org/plugins-archives/maven-jar-plugin-2.6/examples/create-test-jar.html

In order to let Maven resolve all test-scoped transitive dependencies you should create a separate project.

jetersen · 2019-12-07T15:39:25Z

Ya would be great if we could generate the test jar from it's own POM xml

timja · 2019-12-07T16:29:45Z

I tried moving it to it's own module, but got cyclic dependency issues :(

jetersen · 2019-12-07T16:46:21Z

Depend on the test module with an exclude to avoid the cyclic dependency?

timja · 2019-12-07T16:52:40Z

Depend on the test module with an exclude to avoid the cyclic dependency?

Didn't work, I'm moving all the tests that rely on the rule / Util to the test module and will see how that looks

timja · 2019-12-07T17:06:26Z

Pushed, seemed to be working locally, although I didn't wait for a full run.

jetersen · 2019-12-07T17:13:20Z

codacy has some opinion :)

stale

timja · 2019-12-07T17:17:16Z

codacy has some opinion :)

on ancient code I haven't touched (but moved in src from test) 😉

jetersen · 2019-12-07T17:18:03Z

Not true have a look at the changes: cd01391

timja · 2019-12-07T17:19:20Z

Not true have a look at the changes: cd01391

wtf =/, IDE went mad I guess...

jetersen

🥰 Looks good to me! 🐳

I wanted to split it earlier, thanks for your hard work @timja 💪

jetersen · 2019-12-07T18:00:30Z

docs/PLUGINS.md

@@ -212,10 +212,9 @@ Add the Configuration as Code plugin as a test dependency in your pom.xml:
    <scope>test</scope>
 </dependency>
 <dependency>
-    <groupId>io.jenkins</groupId>
-    <artifactId>configuration-as-code</artifactId>
+    <groupId>io.jenkins.configuration-as-code</groupId>


This needs to be documented in the release notes so when dependabot does the PR it is very transparent that this is the change we did (since we have removed the test jar under classifier tests)

timja · 2019-12-07T18:15:08Z

Permissions PR: jenkins-infra/repository-permissions-updater#1359

oleg-nenashev · 2019-12-07T19:37:50Z

Note that it will reduce efficiency of PCT. FYI @batmat @varyvol @alecharp

sladyn98 · 2019-12-07T19:46:27Z

Looks like the dependencies caused a bit of a problem.List of errors would give us more information while debugging. LGTM. Thanks @timja

timja · 2019-12-07T19:54:19Z

Note that it will reduce efficiency of PCT. FYI @BatMar @varyvol @alecharp

How so? I assume it's because plugins won't be using the new dependency and might fail on new jcasc versions.

timja · 2019-12-07T20:19:53Z

I don't really understand why it takes 2hr 30 to build this on jenkins CI and 11 minutes on GitHub actions 😠

timja · 2019-12-09T13:43:08Z

hosting permissions have been updated to allow publishing this module
@oleg-nenashev are you happy with this change?

timja · 2019-12-10T08:52:25Z

Will merge later today if no objections.
cc @oleg-nenashev

oleg-nenashev · 2019-12-10T09:12:45Z

How so? I assume it's because plugins won't be using the new dependency and might fail on new jcasc versions.

Plugin Compatibility Tester invokes tests only in the plugin module. This change moves some of the tests to a different location. IIRC there should have been a bug created for it after JENKINS-58069, but I cannot find it. CC @varyvol

Apart from that, no objection w.r.t the PR itself

oleg-nenashev · 2019-12-10T09:26:51Z

Probably I should have been more explicit. My preference was to wait a bit more to gather feedback from interested parties. But 🤷‍♂ since it was merged, we can revisit it if there is any feedback

oleg-nenashev · 2019-12-10T09:40:50Z

Fixes #917

Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com>

jglick · 2020-01-13T17:34:14Z

plugins won't be using the new dependency and might fail on new jcasc versions

Yes, this is a problem. jenkinsci/bom#164

* Add support to backup and restore automatically * Fix the potential output resource leak * Add configs about deploy into alauda update-center * Set alauda as the main branch * Add sonarqube properties file * Fix the potentital issues * Add a merge strategy extension point for the YAML config (#1) * Add YAML merge strategy * Add a new merge strategy * Remove override merge strategy * Clean the unused code lines * Clean the unused imports * Add test cases for merge strategy * Fix the wrong test cases * Take merge strategy name everytime * Give the default strategy a more readable name * Validate top README with the Integrations tests (jenkinsci#1229) * Fix the compilation error caused by merge strategy * Fix issues which found by spotbugs * Remove unused imports * Move profiles from pom.xml into settings file * Remove empty line * rename the shared-library * Schema Generation for nested yml configurations (jenkinsci#1027) * Trying to diagnose some flaky tests (jenkinsci#1243) * Update ci to include code coverage after jenkinsci#1215 (jenkinsci#1241) Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com> * Checkstyle ignore release.properties (jenkinsci#1245) * [maven-release-plugin] prepare release configuration-as-code-1.35 * [maven-release-plugin] prepare for next development iteration * Bump jackson.version from 2.10.1 to 2.10.2 Bumps `jackson.version` from 2.10.1 to 2.10.2. Updates `jackson-databind` from 2.10.1 to 2.10.2 - [Release notes](https://github.com/FasterXML/jackson/releases) - [Commits](https://github.com/FasterXML/jackson/commits) Updates `jackson-dataformat-yaml` from 2.10.1 to 2.10.2 - [Release notes](https://github.com/FasterXML/jackson-dataformats-text/releases) - [Commits](FasterXML/jackson-dataformats-text@jackson-dataformats-text-2.10.1...jackson-dataformats-text-2.10.2) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> * Correct spelling of hpi.pluginChangelogUrl tag (jenkinsci#1248) * Automatically request code reviews from all JCasC plugin developers (jenkinsci#1249) * [Security] Bump checkstyle from 8.26 to 8.29 Bumps [checkstyle](https://github.com/checkstyle/checkstyle) from 8.26 to 8.29. **This update includes a security fix.** - [Release notes](https://github.com/checkstyle/checkstyle/releases) - [Commits](checkstyle/checkstyle@checkstyle-8.26...checkstyle-8.29) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> * It has been renamed to `master.componentName` (jenkinsci#1256) Co-authored-by: Tim Jacomb <t.jacomb@kainos.com> * Fix the version specification for Jenkins 2.199 (jenkinsci#1268) * Added example for credentials certificate (jenkinsci#1270) * Use the latest shared-library branch * Update test to match latest plugin version (jenkinsci#1273) * Update the integration test for active-directory (jenkinsci#1274) * Add integration test for pipeline-model-definition (jenkinsci#1276) * Fix the error when no casc user dir (#5) * Add missing of mode in the context * Use the same version of jackson-core * Use the same version of jackson-core * Use 2.8 as the major version * Fix the wrong parent id for the plugin releasing * Move the user config into a backup dir * Run build under java pod Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com> Co-authored-by: Sladyn <sladynnunes98@gmail.com> Co-authored-by: Jesse Glick <jglick@cloudbees.com> Co-authored-by: Joseph Petersen <josephp90@gmail.com> Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com> Co-authored-by: Tim Jacomb <t.jacomb@kainos.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Mark Waite <mark.earl.waite@gmail.com> Co-authored-by: Jhon Mike <jhon.msdev@gmail.com> Co-authored-by: Dawid Gosławski <50369002+dg-nvm@users.noreply.github.com> Co-authored-by: choeffer <33625292+choeffer@users.noreply.github.com> Co-authored-by: Francisco Fernández <31063239+fcojfernandez@users.noreply.github.com>

Change json schema test API to return list of errors

a96a4f7

timja requested review from sladyn98, jetersen and oleg-nenashev December 3, 2019 07:52

jetersen previously approved these changes Dec 3, 2019

View reviewed changes

Checkstyle

5e9ce85

sladyn98 reviewed Dec 3, 2019

View reviewed changes

Fix test

a71b71e

timja mentioned this pull request Dec 3, 2019

/reload-configuration-as-code/?casc-reload-token returns a 500 when config contains a amazonEC2 section (but /configuration-as-code/reload does not) #1168

Closed

Introduce test-harness module

cd01391

timja force-pushed the change-json-schema-test-api branch from c76b703 to cd01391 Compare December 7, 2019 17:11

jetersen self-requested a review December 7, 2019 17:15

shush codacy + docs

e50c94f

timja added 2 commits December 7, 2019 17:40

Fix javadoc

55e0216

Fix import

80221ea

timja requested a review from sladyn98 December 7, 2019 17:56

timja changed the title ~~Change json schema test API to return list of errors~~ Add test-harness module + change json schema test API to return list of errors Dec 7, 2019

jetersen approved these changes Dec 7, 2019

View reviewed changes

timja mentioned this pull request Dec 7, 2019

Add upload permissions for configuration-as-code-test-harness jenkins-infra/repository-permissions-updater#1359

Merged

4 tasks

sladyn98 approved these changes Dec 7, 2019

View reviewed changes

timja added chore a PR that adds to maintenance - used by Release Drafter breaking A PR that is a breaking change - used by Release Drafter labels Dec 9, 2019

timja merged commit 6cc682c into jenkinsci:master Dec 10, 2019

timja deleted the change-json-schema-test-api branch December 10, 2019 09:14

oleg-nenashev changed the title ~~Add test-harness module + change json schema test API to return list of errors~~ #917 - Add test-harness module + change json schema test API to return list of errors Dec 10, 2019

timja mentioned this pull request Dec 10, 2019

Introduce a new JCasC Test Harness Library #917

Closed

1 task

jetersen mentioned this pull request Dec 27, 2019

Update ci to include code coverage after #1215 #1241

Merged

6 tasks

timja pushed a commit that referenced this pull request Jan 11, 2020

Update ci to include code coverage after #1215 (#1241)

abfc695

Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com>

jglick mentioned this pull request Jan 13, 2020

Revert "Bump configuration-as-code from 1.34 to 1.35 (test harness renamed)" jenkinsci/bom#164

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#917 - Add test-harness module + change json schema test API to return list of errors #1215

#917 - Add test-harness module + change json schema test API to return list of errors #1215

timja commented Dec 3, 2019 •

edited

timja commented Dec 3, 2019

jetersen left a comment

sladyn98 Dec 3, 2019

timja Dec 3, 2019

timja commented Dec 3, 2019

timja commented Dec 4, 2019

timja commented Dec 4, 2019

jetersen commented Dec 5, 2019

timja commented Dec 5, 2019

timja commented Dec 7, 2019

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019

timja commented Dec 7, 2019

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019 •

edited

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019

jetersen left a comment

jetersen Dec 7, 2019 •

edited

timja commented Dec 7, 2019

oleg-nenashev commented Dec 7, 2019 •

edited by timja

sladyn98 commented Dec 7, 2019 •

edited

timja commented Dec 7, 2019 •

edited

timja commented Dec 7, 2019 •

edited

timja commented Dec 9, 2019

timja commented Dec 10, 2019

oleg-nenashev commented Dec 10, 2019

oleg-nenashev commented Dec 10, 2019

oleg-nenashev commented Dec 10, 2019

jglick commented Jan 13, 2020

#917 - Add test-harness module + change json schema test API to return list of errors #1215

#917 - Add test-harness module + change json schema test API to return list of errors #1215

Conversation

timja commented Dec 3, 2019 • edited

Your checklist for this pull request

timja commented Dec 3, 2019

jetersen left a comment

Choose a reason for hiding this comment

sladyn98 Dec 3, 2019

Choose a reason for hiding this comment

timja Dec 3, 2019

Choose a reason for hiding this comment

timja commented Dec 3, 2019

timja commented Dec 4, 2019

timja commented Dec 4, 2019

jetersen commented Dec 5, 2019

timja commented Dec 5, 2019

timja commented Dec 7, 2019

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019

timja commented Dec 7, 2019

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019 • edited

jetersen commented Dec 7, 2019

timja commented Dec 7, 2019

jetersen left a comment

Choose a reason for hiding this comment

jetersen Dec 7, 2019 • edited

Choose a reason for hiding this comment

timja commented Dec 7, 2019

oleg-nenashev commented Dec 7, 2019 • edited by timja

sladyn98 commented Dec 7, 2019 • edited

timja commented Dec 7, 2019 • edited

timja commented Dec 7, 2019 • edited

timja commented Dec 9, 2019

timja commented Dec 10, 2019

oleg-nenashev commented Dec 10, 2019

oleg-nenashev commented Dec 10, 2019

oleg-nenashev commented Dec 10, 2019

jglick commented Jan 13, 2020

timja commented Dec 3, 2019 •

edited

timja commented Dec 7, 2019 •

edited

jetersen Dec 7, 2019 •

edited

oleg-nenashev commented Dec 7, 2019 •

edited by timja

sladyn98 commented Dec 7, 2019 •

edited

timja commented Dec 7, 2019 •

edited

timja commented Dec 7, 2019 •

edited