Skip to content

Commit da9ad88

Browse files
committed
Prevent testProviderRefresh test failures due to log ordering differences
Accept either the typical ordering of log messages or an alternate ordering of log messages in testProviderRefresh. The same messages are used in each case, but the sequence of messages changes. Accepting either ordering assures that we won't have plugin BOM failures due to timing differences or other differences in test infrastructure. jenkinsci/bom#5718 (comment) detected a failure in this test when run within the plugin bill of materials. jenkinsci/bom#5720 (comment) tested more deeply and found that it failed more than 5% of my test runs locally. Additional local testing confirmed that the order of the log messages was sometimes slightly different than the original assertion expected. This change accepts either ordering. Testing done: * Before this change, the test would fail 5% of the time on my AMD Ryzen 5 5600X 6-Core Processor on Red Hat Enterprise Linux 8 when run with `mvn -Dtest=GithubAppCredentialsTest#testProviderRefresh test` * After this change, the test passes 100% of the time in the 36 test runs that I used to check it
1 parent db846d7 commit da9ad88

File tree

1 file changed

+50
-19
lines changed

1 file changed

+50
-19
lines changed

src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsTest.java

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -287,27 +287,58 @@ public void testProviderRefresh() throws Exception {
287287

288288
List<String> credentialsLog = getOutputLines();
289289

290+
// Assert that individual messages occur at least once in the credentials log
291+
String generatingMsg = "Generating App Installation Token for app ID 54321";
292+
String failedMsg =
293+
"Failed to generate new GitHub App Installation Token for app ID 54321: cached token is stale but has not expired";
294+
295+
// Be sure the expected messages are in the log
296+
assertThat(credentialsLog, hasItem(generatingMsg));
297+
assertThat(credentialsLog, hasItem(failedMsg));
298+
290299
// Verify correct messages from GitHubAppCredential logger indicating token was retrieved on
291300
// agent
292-
assertThat(
293-
"Creds should cache on master",
294-
credentialsLog,
295-
contains(
296-
// refresh on controller
297-
"Generating App Installation Token for app ID 54321",
298-
// next call uses cached token
299-
// sleep and then refresh stale token
300-
"Generating App Installation Token for app ID 54321",
301-
// next call (error forced by wiremock)
302-
"Failed to generate new GitHub App Installation Token for app ID 54321: cached token is stale but has not expired",
303-
// next call refreshes the still stale token
304-
"Generating App Installation Token for app ID 54321",
305-
// sleep and then refresh stale token hits another error forced by wiremock
306-
"Failed to generate new GitHub App Installation Token for app ID 54321: cached token is stale but has not expired",
307-
// next call refreshes the still stale token
308-
"Generating App Installation Token for app ID 54321"
309-
// next call uses cached token
310-
));
301+
if (credentialsLog.get(2).equals(failedMsg)) {
302+
assertThat(
303+
"Creds should cache on master - typical order",
304+
credentialsLog,
305+
contains(
306+
// refresh on controller
307+
generatingMsg,
308+
// next call uses cached token
309+
// sleep and then refresh stale token
310+
generatingMsg,
311+
// next call (error forced by wiremock)
312+
failedMsg,
313+
// next call refreshes the still stale token
314+
generatingMsg,
315+
// sleep and then refresh stale token hits another error forced by wiremock
316+
failedMsg,
317+
// next call refreshes the still stale token
318+
generatingMsg
319+
// next call uses cached token
320+
));
321+
} else {
322+
assertThat(
323+
"Creds should cache on master - alternate order",
324+
credentialsLog,
325+
contains(
326+
// refresh on controller
327+
generatingMsg,
328+
// next call uses cached token
329+
// sleep and then refresh stale token
330+
generatingMsg,
331+
// next call refreshes the still stale token
332+
generatingMsg,
333+
// next call (error forced by wiremock)
334+
failedMsg,
335+
// sleep and then refresh stale token hits another error forced by wiremock
336+
failedMsg,
337+
// next call refreshes the still stale token
338+
generatingMsg
339+
// next call uses cached token
340+
));
341+
}
311342

312343
// Getting the token for via AuthorizationProvider on controller should not check rate_limit
313344
githubApi.verify(

0 commit comments

Comments
 (0)