Cleanup tests

[res0nance]

• Cleanup some tests with using the older now NOOPed doYank() method
• Rewrite the delay calculation in RateLimitBranchPropertyTest to not be truncated.
• Fix the delay to not be an overly bloated number

Running it in CI to try to reproduce all the test flakes

[res0nance]

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

[res0nance]

I tested this locally and i got a mind boggling result

[ERROR] Failures:
[ERROR]   RateLimitBranchPropertyTest.rateLimitsBlockBuilds_maxRate:133->rateLimitsBlockBuilds:201 At least the rate implied delay but no more than 500ms longer
Expected: (a value equal to or greater than <3400L> and a value less than or equal to <4100L>)
     but: a value equal to or greater than <3400L> <1718L> was less than <3400L>
[ERROR]   RateLimitBranchPropertyTest.rateLimitsBlockBuilds_medRate:138->rateLimitsBlockBuilds:201 At least the rate implied delay but no more than 500ms longer
Expected: (a value equal to or greater than <7000L> and a value less than or equal to <7700L>)
     but: a value equal to or greater than <7000L> <5332L> was less than <7000L>
[INFO]
[ERROR] Tests run: 10, Failures: 2, Errors: 0, Skipped: 0

[res0nance]

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))
	);
	}
	}

[bitwiseman]

I watch with interest...

[res0nance]

I watch with interest...

Now i always run into a different error. 😅

[res0nance]

Huzzah it worked

[bitwiseman]

Okay, so you got it to succeed once. Does this mean the tests aren't flaky now?

[res0nance]

It shouldn't be flaky as it stopped flaking on my local machine the one with

[ERROR] Failures:
[ERROR]   RateLimitBranchPropertyTest.rateLimitsBlockBuilds_maxRate:133->rateLimitsBlockBuilds:201 At least the rate implied delay but no more than 500ms longer
Expected: (a value equal to or greater than <3400L> and a value less than or equal to <4100L>)
     but: a value equal to or greater than <3400L> <1718L> was less than <3400L>
[ERROR]   RateLimitBranchPropertyTest.rateLimitsBlockBuilds_medRate:138->rateLimitsBlockBuilds:201 At least the rate implied delay but no more than 500ms longer
Expected: (a value equal to or greater than <7000L> and a value less than or equal to <7700L>)
     but: a value equal to or greater than <7000L> <5332L> was less than <7000L>
[INFO]
[ERROR] Tests run: 10, Failures: 2, Errors: 0, Skipped: 0

So at least for this flake it should stop, it was root caused in my previous comment as well. So I expect the flaking for this particular test to stop