Ext_Proc : Adding integration test with MxN streaming enabled in both directions

[yanjunxiang-google]

Ext_Proc : Adding integration test with MxN streaming enabled in both directions

fixes issue: #37770

[yanjunxiang-google]

/assign @yanavlasov @tyxia

[yanjunxiang-google]

/assign @jmarantz

[jmarantz]

flushing comments.

[jmarantz]

A pattern I use sometimes when looking at expectations in a helper function is to provide more context by doing << something in the EXPECT call. That way, if you have multiple calls to it from the same test method, and one of them fails, you'll know which one. You might pass a string for something into the function, or compute something from the args.

[yanjunxiang-google]

For waitForHttpConnection() and waitForNewStream(), these functions are already returning those "something", and printed out in the log, for example:

envoy/test/integration/fake_upstream.cc

Line 798 in 66cc217

return AssertionFailure() << "Timed out waiting for new connection.";

.

I added << "something in the below two places.

[jmarantz]

remove the ! and flip the then/else clauses.

[yanjunxiang-google]

done

[jmarantz]

ditto

[yanjunxiang-google]

done

[yanavlasov]

/wait

[jmarantz]

you missed the point of what I was suggesting. If there's a failure, it will give the line number of the EXPECT but not the stack.

You have multiple call-sites to this function, so to disambiugate a failure you need to pass something from those call-sites to then put in the EXPECT annotation.

[yanjunxiang-google]

If there is a test failure in these integration tests, the way I debug this is running the test with the trace logs enabled, which will give us the entire ext_proc state machine change sequence. And the error will be obvious by checking the logs in the context.

[yanjunxiang-google]

For example, If I deliberately cause an error here, below are the logs are I saw. The logs give us enough info to know what's wrong:

[2024-12-20 19:02:23.127][16][debug][testing] [./test/integration/fake_upstream.h:182] Received gRPC message: request_body {
body: "ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss"
}
metadata_context {
}

test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc:778: Failure
Value of: body_request.has_request_body()
Actual: true
Expected: false

Stack trace:
0x3c12563: Envoy::ExtProcIntegrationTest::serverReceiveBodyDuplexStreamed()
0x3ac81c7: Envoy::ExtProcIntegrationTest_ServerWaitForBodyBeforeSendsHeaderRespDuplexStreamed_Test::TestBody()
0x9321bfb: testing::internal::HandleSehExceptionsInMethodIfSupported<>()
0x9311dfa: testing::internal::HandleExceptionsInMethodIfSupported<>()
0x92fdcd3: testing::Test::Run()
0x92fe684: testing::TestInfo::Run()
... Google Test internal frames ...

[2024-12-20 19:02:23.127][16][debug][testing] [./test/integration/fake_upstream.h:190] Waiting for gRPC message...

[jmarantz]

I was trying to suggest a pattern that would make test failures easier to diagnose even if you don't have deep knowledge.

It's not essential -- you could also use a debugger to figure out where the failure was. I just thought it might be easier for others without deep context to debug if you made it explicit.