test(dialogflow): deflake and re-enable test cases in ITSystemTest

[whowes]

Each of these test cases failed 7/30 times locally without these fixes. With the fixes applied, tests pass without flakes in 100+ runs.

• detectIntentTest() flaked regularly if the intent created in setUp() didn't have sufficient time to fully propagate before detectIntent() was called in the test case. Poll until detectIntent() returns a non-empty result before proceeding to assertion on the content.

• listContextsTest() flaked if a system context was encountered (see https://docs.cloud.google.com/dialogflow/es/docs/reference/rest/v2beta1/projects.agent.environments.users.sessions.contexts/patch#path-parameters). Adjust the flaky assertion to only look for the expected context while tolerating others that happen to be present.

[gemini-code-assist]

Code Review

This pull request enables previously ignored integration tests (detectIntentTest and listContextsTest) in ITSystemTest and introduces the awaitility library to handle eventual consistency. The feedback suggests improving the robustness of these tests against flakiness. Specifically, for detectIntentTest, it is recommended to use untilAsserted to poll and retry all assertions together, which also removes the need for AtomicReference boilerplate. For listContextsTest, wrapping the assertion in an awaitility block is advised to prevent failures caused by replication lag.

[gemini-code-assist]

Using until(...) with a condition that only checks if queryText is not empty might not actually prevent flakiness. Since the query input is an event, the query text in the response is immediately populated with the event name on the first attempt, even if the intent itself hasn't been fully propagated or matched yet. This causes the await() block to exit immediately, and the subsequent assertions (like matching the intent display name) might still fail due to eventual consistency.

Instead, use untilAsserted(...) to poll and retry all assertions. This also eliminates the need for the AtomicReference boilerplate.

Suggested change

	AtomicReference<DetectIntentResponse> responseRef =
	new AtomicReference<>();
	await()
	.atMost(Duration.ofSeconds(10))
	.pollInterval(Duration.ofSeconds(2))
	.until(
	() -> {
	DetectIntentResponse response = sessionsClient.detectIntent(request);
	responseRef.set(response);
	return !response.getQueryResult().getQueryText().isEmpty();
	});
	QueryResult result = responseRef.get().getQueryResult();
	assertEquals(EVENT_NAME, result.getQueryText());
	assertEquals(ACTION_NAME, result.getAction());
	assertEquals(DEFAULT_LANGUAGE_CODE, result.getLanguageCode());
	assertEquals(intent.getDisplayName(), result.getIntent().getDisplayName());
	await()
	.atMost(Duration.ofSeconds(10))
	.pollInterval(Duration.ofSeconds(2))
	.untilAsserted(
	() -> {
	DetectIntentResponse response = sessionsClient.detectIntent(request);
	QueryResult result = response.getQueryResult();
	assertEquals(EVENT_NAME, result.getQueryText());
	assertEquals(ACTION_NAME, result.getAction());
	assertEquals(DEFAULT_LANGUAGE_CODE, result.getLanguageCode());
	assertEquals(intent.getDisplayName(), result.getIntent().getDisplayName());
	});

[jinseopkim0]

Is this relevant?

[whowes]

I wasn't able to repro any flakiness with my original change (not retrying on failed assertions), but this seems like a reasonable improvement since Gemini pointed out that the query text will be populated with the event name (thus not empty) even if processing is not yet complete.

[gemini-code-assist]

To ensure listContextsTest is fully deflaked against eventual consistency (replication lag), we should also wrap the assertion in an awaitility block. Otherwise, if the context is not immediately visible in the list, the test will fail.

Suggested change

	boolean contextNameInActualContext =
	stream(contextsClient.listContexts(request).iterateAll())
	.anyMatch(actualContext -> context.getName().equals(actualContext.getName()));
	assertTrue(contextNameInActualContext);
	await()
	.atMost(Duration.ofSeconds(10))
	.pollInterval(Duration.ofSeconds(2))
	.untilAsserted(
	() -> {
	boolean contextNameInActualContext =
	stream(contextsClient.listContexts(request).iterateAll())
	.anyMatch(actualContext -> context.getName().equals(actualContext.getName()));
	assertTrue(contextNameInActualContext);
	});

[jinseopkim0]

Is this relevant?

[whowes]

I don't think this one is necessary - the observed flakiness was related to the unexpected system context appearing in the list and not related to eventual consistency.

[jinseopkim0]

Would it make sense to replace this with the following?

.until(
        () -> sessionsClient.detectIntent(request),
        res -> !res.getQueryResult().getQueryText().isEmpty());

[whowes]

I opted to move to the untilAsserted() approach from the Gemini suggestion instead.

[jinseopkim0]

Is this relevant?