Do not load detached plugins during tests

[jglick]

Downstream of jenkinsci/jenkins-test-harness#54.

Description

While running some core functional tests in an environment with extremely slow filesystem access (Windows over VirtualBox; some driver problem I suppose), I noticed that each test case seemed to spend several minutes during initialization in createClassJarFromWebInfClasses, especially in subversion.jpi, but also in several other smaller plugins.

Now classes.jar is generated at build time for any plugin built in the past four years or so, but to allow Remoting to cache JARs for really old plugin binaries, we do this step at runtime if necessary. I added a warning when that was necessary.

But why was every functional test loading subversion-1.54.jpi and the like, anyway? These are detached plugins as of Jenkins 2, not normally used except when needed during upgrades. It turns out the test harness, unlike the real plugin manager, was loading all detached plugins. This just seems like a waste of CPU & I/O.

Changelog entries

None required.

Desired reviewers

@reviewbybees, @jenkinsci/code-reviewers

[ghost]

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

[oleg-nenashev]

Would be nice to understand how it will work together with #2754, which will likely require detached plugins at some point (e.g. moving the master-side Remoting protocol specifics to a remoting plugin). I would expect such (System?) detached plugins to be always loaded

[jglick]

#2754 […] will likely require detached plugins

Not that I know of. Anyway this PR is only about tests, not production mode.

[olivergondza]

I am +1 on the idea. On a brief look 4/5 test failures are related, though.

[jglick]

Indeed it looks like this needs some test fixes.

[jglick]

The unrelated failures are covered by INFRA-1141.

[jglick]

Still have a test failure

org.junit.ComparisonFailure: expected:<...64)"},{"executors":1[,"os":"Linux (amd64)"]}]> but was:<...64)"},{"executors":1[]}]>
	at org.junit.Assert.assertEquals(Assert.java:115)
	at org.junit.Assert.assertEquals(Assert.java:144)
	at hudson.model.UsageStatisticsTest.compareWithFile(UsageStatisticsTest.java:175)
	at hudson.model.UsageStatisticsTest.roundtrip(UsageStatisticsTest.java:121)

[oleg-nenashev]

Kinda 🐜 for the upstream jenkinsci/jenkins-test-harness#54 , which breaks compatibility of JTH by removing TestPluginManager#installResourcePlugin . OTOH I do not see any usages of this method.

This PR looks good, but we need to agree what Binary compat requirements do we declare for JTH

[oleg-nenashev]

I still have not created a PR with logging improvements :(

[jglick]

breaks compatibility of JTH by removing TestPluginManager#installResourcePlugin

It was used only in the code I am deleting here.

we need to agree what Binary compat requirements do we declare for JTH

We make no guarantees about binary compatibility of test utilities.

[jglick]

The test failures are still INFRA-1141.

[oleg-nenashev]

Not fully convinced about test utilities, but I agree it is not mentioned anywhere. Hence 🐝

[jglick]

@reviewbybees done

[oleg-nenashev]

Will merge it after the weekly release

[jglick]

Why after? There is no testing that will happen in the next week that did not happen in the previous week. Just wondering about the hesitation over merging a test-only PR.

[oleg-nenashev]

Just because we already had issues with parallel PR collisions. I didn't want to merge it closely to the weekly. Merging now