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.

Sign up for GitHub

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

Cleanup tests #177

Merged
merged 6 commits into from
Feb 13, 2020
Merged

Cleanup tests #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a576015
Cleanup tests
Jan 31, 2020
cc57948
Add missing trait
Jan 31, 2020
499ed2b
Cleanup unused imports
res0nance Feb 2, 2020
65a247c
Cleanup and fix startTime
Feb 6, 2020
33d57e5
Merge branch 'test-fixes' of https://github.com/res0nance/branch-api-…
Feb 6, 2020
18eaf8e
Fix compile error
Feb 6, 2020
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
14 changes: 0 additions & 14 deletions src/test/java/jenkins/branch/BranchNameContributorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@
package jenkins.branch;

import hudson.EnvVars;
import hudson.model.Computer;
import hudson.model.EnvironmentContributor;
import hudson.model.Executor;
import hudson.model.FreeStyleProject;
import hudson.model.TopLevelItem;
import hudson.util.LogTaskListener;
Expand Down Expand Up @@ -69,18 +67,6 @@ public void cleanOutAllItems() throws Exception {
for (TopLevelItem i : r.getInstance().getItems()) {
i.delete();
}
for (Computer comp : r.jenkins.getComputers()) {
for (Executor e : comp.getExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
for (Executor e : comp.getOneOffExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
}
}

@Test
Expand Down
9 changes: 8 additions & 1 deletion src/test/java/jenkins/branch/NoTriggerBranchPropertyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,13 @@
import jenkins.branch.harness.MultiBranchImpl;
import jenkins.plugins.git.GitSCMSource;
import jenkins.plugins.git.GitSampleRepoRule;
import jenkins.plugins.git.traits.BranchDiscoveryTrait;

import org.junit.Test;
import static org.junit.Assert.*;

import java.util.Collections;

import org.junit.Rule;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
Expand All @@ -59,7 +64,9 @@ public void smokes() throws Exception {
sampleRepo.git("add", "server");
sampleRepo.git("commit", "--all", "--message=release");
MultiBranchImpl stuff = r.jenkins.createProject(MultiBranchImpl.class, "stuff");
BranchSource branchSource = new BranchSource(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false));
GitSCMSource source = new GitSCMSource(sampleRepo.toString());
source.setTraits(Collections.singletonList(new BranchDiscoveryTrait()));
BranchSource branchSource = new BranchSource(source);
branchSource.setStrategy(new NamedExceptionsBranchPropertyStrategy(new BranchProperty[0], new NamedExceptionsBranchPropertyStrategy.Named[] {
new NamedExceptionsBranchPropertyStrategy.Named("release*", new BranchProperty[] {new NoTriggerBranchProperty()})
}));
Expand Down
6 changes: 4 additions & 2 deletions src/test/java/jenkins/branch/NoTriggerOrganizationFolderPropertyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import jenkins.branch.harness.MultiBranchImpl;
import jenkins.plugins.git.GitSCMSource;
import jenkins.plugins.git.GitSampleRepoRule;
import jenkins.scm.api.SCMSource;
import jenkins.plugins.git.traits.BranchDiscoveryTrait;
import jenkins.scm.impl.SingleSCMNavigator;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -67,7 +67,9 @@ public void smokes() throws Exception {
assertEquals(".*", prop.getBranches());
top.getProperties().replace(new NoTriggerOrganizationFolderProperty("(?!release.*).*"));
top.getProjectFactories().add(new OrganizationFolderTest.MockFactory());
top.getNavigators().add(new SingleSCMNavigator("stuff", Collections.<SCMSource>singletonList(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false))));
GitSCMSource source = new GitSCMSource(sampleRepo.toString());
source.setTraits(Collections.singletonList(new BranchDiscoveryTrait()));
top.getNavigators().add(new SingleSCMNavigator("stuff", Collections.singletonList(source)));
r.configRoundtrip(top);
prop = top.getProperties().get(NoTriggerOrganizationFolderProperty.class);
assertNotNull(prop);
Expand Down
14 changes: 0 additions & 14 deletions src/test/java/jenkins/branch/OrganizationChildHealthMetricsPropertyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@

import com.cloudbees.hudson.plugins.folder.health.FolderHealthMetric;
import com.cloudbees.hudson.plugins.folder.health.FolderHealthMetricDescriptor;
import hudson.model.Computer;
import hudson.model.Executor;
import hudson.model.TopLevelItem;
import hudson.util.DescribableList;
import integration.harness.BasicMultiBranchProject;
Expand Down Expand Up @@ -66,18 +64,6 @@ public void cleanOutAllItems() throws Exception {
for (TopLevelItem i : r.getInstance().getItems()) {
i.delete();
}
for (Computer comp : r.jenkins.getComputers()) {
for (Executor e : comp.getExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
for (Executor e : comp.getOneOffExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
}
}

@Test
Expand Down
14 changes: 0 additions & 14 deletions src/test/java/jenkins/branch/OrganizationChildOrphanedItemsPropertyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
package jenkins.branch;

import com.cloudbees.hudson.plugins.folder.computed.DefaultOrphanedItemStrategy;
import hudson.model.Computer;
import hudson.model.Executor;
import hudson.model.TopLevelItem;
import integration.harness.BasicMultiBranchProject;
import integration.harness.BasicMultiBranchProjectFactory;
Expand Down Expand Up @@ -62,18 +60,6 @@ public void cleanOutAllItems() throws Exception {
for (TopLevelItem i : r.getInstance().getItems()) {
i.delete();
}
for (Computer comp : r.jenkins.getComputers()) {
for (Executor e : comp.getExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
for (Executor e : comp.getOneOffExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
}
}

@Test
Expand Down
14 changes: 0 additions & 14 deletions src/test/java/jenkins/branch/OrganizationChildTriggersPropertyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
package jenkins.branch;

import com.cloudbees.hudson.plugins.folder.computed.PeriodicFolderTrigger;
import hudson.model.Computer;
import hudson.model.Executor;
import hudson.model.TopLevelItem;
import hudson.triggers.Trigger;
import hudson.triggers.TriggerDescriptor;
Expand Down Expand Up @@ -68,18 +66,6 @@ public void cleanOutAllItems() throws Exception {
for (TopLevelItem i : r.getInstance().getItems()) {
i.delete();
}
for (Computer comp : r.jenkins.getComputers()) {
for (Executor e : comp.getExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
for (Executor e : comp.getOneOffExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
}
}

@Test
Expand Down
13 changes: 10 additions & 3 deletions src/test/java/jenkins/branch/OverrideIndexTriggersJobPropertyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import jenkins.branch.harness.MultiBranchImpl;
import jenkins.plugins.git.GitSCMSource;
import jenkins.plugins.git.GitSampleRepoRule;
import jenkins.plugins.git.traits.BranchDiscoveryTrait;
import jenkins.scm.api.SCMSource;
import jenkins.scm.impl.SingleSCMNavigator;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -62,7 +63,9 @@ public void overridesDisabledBranch() throws Exception {
sampleRepo.git("add", "server");
sampleRepo.git("commit", "--all", "--message=release");
MultiBranchImpl stuff = r.jenkins.createProject(MultiBranchImpl.class, "stuff");
BranchSource branchSource = new BranchSource(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false));
GitSCMSource source = new GitSCMSource(sampleRepo.toString());
source.setTraits(Collections.singletonList(new BranchDiscoveryTrait()));
BranchSource branchSource = new BranchSource(source);
branchSource.setStrategy(new NamedExceptionsBranchPropertyStrategy(new BranchProperty[0], new NamedExceptionsBranchPropertyStrategy.Named[] {
new NamedExceptionsBranchPropertyStrategy.Named("release*", new BranchProperty[] {new NoTriggerBranchProperty()})
}));
Expand Down Expand Up @@ -128,7 +131,9 @@ public void overridesDisabledOrg() throws Exception {
assertEquals(".*", prop.getBranches());
top.getProperties().replace(new NoTriggerOrganizationFolderProperty("(?!release.*).*"));
top.getProjectFactories().add(new OrganizationFolderTest.MockFactory());
top.getNavigators().add(new SingleSCMNavigator("stuff", Collections.<SCMSource>singletonList(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false))));
GitSCMSource source = new GitSCMSource(sampleRepo.toString());
source.setTraits(Collections.singletonList(new BranchDiscoveryTrait()));
top.getNavigators().add(new SingleSCMNavigator("stuff", Collections.singletonList(source)));
r.configRoundtrip(top);
prop = top.getProperties().get(NoTriggerOrganizationFolderProperty.class);
assertNotNull(prop);
Expand Down Expand Up @@ -188,7 +193,9 @@ public void overridesEnabled() throws Exception {
sampleRepo.git("add", "stuff");
sampleRepo.git("commit", "--all", "--message=master-1");
MultiBranchImpl stuff = r.jenkins.createProject(MultiBranchImpl.class, "stuff");
BranchSource branchSource = new BranchSource(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false));
GitSCMSource source = new GitSCMSource(sampleRepo.toString());
source.setTraits(Collections.singletonList(new BranchDiscoveryTrait()));
BranchSource branchSource = new BranchSource(source);
stuff.getSourcesList().add(branchSource);
r.configRoundtrip(stuff);
stuff.scheduleBuild2(0).getFuture().get();
Expand Down
14 changes: 0 additions & 14 deletions src/test/java/jenkins/branch/PrimaryBranchHealthMetricTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package jenkins.branch;

import hudson.model.Computer;
import hudson.model.Executor;
import hudson.model.FreeStyleProject;
import hudson.model.TopLevelItem;
import integration.harness.BasicMultiBranchProject;
Expand Down Expand Up @@ -30,18 +28,6 @@ public void cleanOutAllItems() throws Exception {
for (TopLevelItem i : r.getInstance().getItems()) {
i.delete();
}
for (Computer comp : r.jenkins.getComputers()) {
for (Executor e : comp.getExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
for (Executor e : comp.getOneOffExecutors()) {
if (e.getCauseOfDeath() != null) {
e.doYank();
}
}
}
}

@Test
Expand Down
24 changes: 14 additions & 10 deletions src/test/java/jenkins/branch/RateLimitBranchPropertyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
Expand Down Expand Up @@ -169,13 +170,14 @@ public void rateLimitsBlockBuilds(int rate) throws Exception {
);
assertThat(master.isInQueue(), is(false));
assertThat(master.getQueueItem(), nullValue());
assertThat(master.getBuilds().getLastBuild(), notNullValue());
long startTime = master.getBuilds().getLastBuild().getTimeInMillis();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably why it flakes all the time lastBuild is not null so it uses the time of the last build to throttle

branch-api-plugin/src/main/java/jenkins/branch/RateLimitBranchProperty.java

Lines 507 to 530 in a1c604b

Run lastBuild = job.getLastBuild();
if (lastBuild != null) {
// we don't mind if the project type allows concurrent builds
// we assume that the last build was started at a time that respects the rate
// thus the next build to start will start such that the time between starts matches the rate
// it doesn't matter if the duration of the build is longer than the time between starts
// only that we linearise the starts and keep a consistent minimum duration between *starts*
long timeSinceLastBuild = System.currentTimeMillis() - lastBuild.getTimeInMillis();
long betweenBuilds = property.getMillisecondsBetweenBuilds();
if (timeSinceLastBuild < betweenBuilds) {
LOGGER.log(Level.FINE, "{0} will be delayed for another {1}ms as it is {2}ms since "
+ "last build and ideal rate is {3}ms between builds",
new Object[]{
job.getFullName(),
betweenBuilds - timeSinceLastBuild,
timeSinceLastBuild,
betweenBuilds
}
);
return CauseOfBlockage.fromMessage(Messages._RateLimitBranchProperty_BuildBlocked(
new Date(lastBuild.getTimeInMillis() + betweenBuilds))
);
}
}

QueueTaskFuture<FreeStyleBuild> future = master.scheduleBuild2(0);

// let the item get added to the queue
while (!master.isInQueue()) {
Thread.yield();
}
long startTime = System.currentTimeMillis();
assertThat(master.isInQueue(), is(true));

// while it is in the queue, until queue maintenance takes place, it will not be flagged as blocked
Expand All @@ -197,11 +199,12 @@ public void rateLimitsBlockBuilds(int rate) throws Exception {
assertThat(startCondition.isDone(), is(true));
// it can take more than the requested delay... that's ok, but it should not be
// more than 500ms longer (i.e. 5 of our Queue.maintain loops above)
final long delay = (long)(60.f * 60.f / rate * 1000);
assertThat("At least the rate implied delay but no more than 500ms longer",
System.currentTimeMillis() - startTime,
allOf(
greaterThanOrEqualTo(60 * 60 / rate * 1000L - 200L),
lessThanOrEqualTo(60 * 60 / rate * 1000L * 500L)
greaterThanOrEqualTo(delay - 200L),
lessThanOrEqualTo(delay + 500L)
)
);
future.get();
Expand All @@ -217,7 +220,7 @@ public void rateLimitsConcurrentBuilds() throws Exception {
prj.setCriteria(null);
BranchSource source = new BranchSource(new MockSCMSource(c, "foo", new MockSCMDiscoverBranches()));
BasicParameterDefinitionBranchProperty p = new BasicParameterDefinitionBranchProperty();
p.setParameterDefinitions(Collections.<ParameterDefinition>singletonList(new StringParameterDefinition("FOO", "BAR")));
p.setParameterDefinitions(Collections.singletonList(new StringParameterDefinition("FOO", "BAR")));
source.setStrategy(new DefaultBranchPropertyStrategy(new BranchProperty[]{
new RateLimitBranchProperty(rate, "hour", false),
new ConcurrentBuildBranchProperty(),
Expand All @@ -244,7 +247,7 @@ public void rateLimitsConcurrentBuilds() throws Exception {
QueueTaskFuture<FreeStyleBuild> future = master.scheduleBuild2(0);
QueueTaskFuture<FreeStyleBuild> future2 = master.scheduleBuild2(0, (Cause) null,
(Action) new ParametersAction(
Collections.<ParameterValue>singletonList(new StringParameterValue("FOO", "MANCHU"))));
Collections.singletonList(new StringParameterValue("FOO", "MANCHU"))));
assertThat(future, not(is(future2)));

// let the item get added to the queue
Expand Down Expand Up @@ -277,7 +280,7 @@ public void rateLimitsConcurrentBuilds() throws Exception {
// will pick up more responsively than the default 5s
startCondition = future2.getStartCondition();
long endTime = startTime + 60*60/rate*1000L*5; // at least 5 times the expected delay
while (!startCondition.isDone() && System.currentTimeMillis() < midTime) {
while (!startCondition.isDone() && System.currentTimeMillis() < endTime) {
Queue.getInstance().maintain();
Thread.sleep(100);
}
Expand All @@ -286,11 +289,12 @@ public void rateLimitsConcurrentBuilds() throws Exception {
FreeStyleBuild secondBuild = startCondition.get();
// it can take more than the requested delay... that's ok, but it should not be
// more than 500ms longer (i.e. 5 of our Queue.maintain loops above)
final long delay = (long)(60.f * 60.f / rate * 1000);
assertThat("At least the rate implied delay but no more than 500ms longer",
secondBuild.getStartTimeInMillis() - firstBuild.getStartTimeInMillis(),
allOf(
greaterThanOrEqualTo(60 * 60 / rate * 1000L - 200L),
lessThanOrEqualTo(60 * 60 / rate * 1000L * 500L)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The * in the less than condition from the comment seems to be a typo

greaterThanOrEqualTo(delay - 200L),
lessThanOrEqualTo(delay + 500L)
)
);
future.get();
Expand All @@ -306,7 +310,7 @@ public void rateLimitsUserBoost() throws Exception {
prj.setCriteria(null);
BranchSource source = new BranchSource(new MockSCMSource(c, "foo", new MockSCMDiscoverBranches()));
BasicParameterDefinitionBranchProperty p = new BasicParameterDefinitionBranchProperty();
p.setParameterDefinitions(Collections.<ParameterDefinition>singletonList(new StringParameterDefinition("FOO", "BAR")));
p.setParameterDefinitions(Collections.singletonList(new StringParameterDefinition("FOO", "BAR")));
source.setStrategy(new DefaultBranchPropertyStrategy(new BranchProperty[]{
new RateLimitBranchProperty(1, "hour", true),
p
Expand Down Expand Up @@ -378,7 +382,7 @@ public void rateLimitsUserBoost() throws Exception {
// now we trigger a user build... it should skip the queue
QueueTaskFuture<FreeStyleBuild> future2 = master.scheduleBuild2(0, new Cause.UserIdCause(),
(Action) new ParametersAction(
Collections.<ParameterValue>singletonList(new StringParameterValue("FOO", "MANCHU"))));
Collections.singletonList(new StringParameterValue("FOO", "MANCHU"))));
// now we wait for the start... invoking queue maintain every 100ms so that the queue
// will pick up more responsively than the default 5s
LOGGER.info("Checking user submitted build skips");
Expand Down