Update Nexus tests to use parallelsuite#9768
Conversation
80aa537 to
c4b319a
Compare
5181ec8 to
04a24f0
Compare
ef22c5f to
5a69e6c
Compare
5a69e6c to
08fc335
Compare
08fc335 to
8a18d8f
Compare
8a18d8f to
04e3d00
Compare
| name string | ||
| outcome string | ||
| endpoint *nexuspb.Endpoint | ||
| endpointName string |
There was a problem hiding this comment.
since we're creating dedicated env per subtest for isolated metric captures, the endpoint needs to be created inside the subtest itself, so just having the name as the field here so each subtest can do
env.createNexusEndpoint(testcore.RandomizeStr("test-endpoint"), testcore.RandomizeStr("task-queue")),
f5e4d87 to
9f7951a
Compare
| ctx := testcore.NewContext() | ||
|
|
||
| s.Run("NamespaceNotFound", func() { | ||
| env.Run("NamespaceNotFound", func() { |
There was a problem hiding this comment.
I don't know if I missed it before, but this needs to be s.Run (otherwise it won't run in parallel). And that will likely require env to be once in every Run to make the metric snapshots deterministic.
| @@ -2690,8 +2703,8 @@ func (s *NexusWorkflowTestSuite) TestNexusAsyncOperationWithMultipleCallers() { | |||
| } | |||
|
|
|||
| for _, tc := range testCases { | |||
| s.Run(tc.input, func() { | |||
| run, err := s.SdkClient().ExecuteWorkflow( | |||
| env.Run(tc.input, func() { | |||
| // Set URL template after httpAPAddress is set, see commonnexus.RouteCompletionCallback | ||
| // We use the FunctionalTestBase.OverrideDynamicConfig(...) here so we can set this global | ||
| // setting even on dedicated cluster setup. | ||
| env.FunctionalTestBase.OverrideDynamicConfig( | ||
| nexusoperations.CallbackURLTemplate, | ||
| "http://"+env.HttpAPIAddress()+"/namespaces/{{.NamespaceName}}/nexus/callback") | ||
|
|
There was a problem hiding this comment.
is there a way to modify this config after a dedicated cluster has been allocated ( in the test)?
I commented this out and only a few tests failed.
There was a problem hiding this comment.
Note we had a previous convo around this here: #9768 (comment)
tl;dr we want to make this a default to avoid having to mark it as dedicated when this default setting works just fine as it's a global config - it should be overridable.
13a41d4 to
6793cd6
Compare
| name string | ||
| onAuthorize func(context.Context, *authorization.Claims, *authorization.CallTarget) (authorization.Result, error) | ||
| checkFailure func(t *testing.T, handlerErr *nexus.HandlerError) | ||
| onAuthorize func(endpointName string) func(context.Context, *authorization.Claims, *authorization.CallTarget) (authorization.Result, error) |
There was a problem hiding this comment.
endpoint name needs to be created in the subtest after env is created, hence needing a factory fn here
44c446f to
3c63355
Compare
3c63355 to
96801fc
Compare
stephanos
left a comment
There was a problem hiding this comment.
LGTM! Please address the remaining comments before you merge.
96801fc to
fa17d8c
Compare
fa17d8c to
15fbf02
Compare
What changed?
NexusTestBaseSuiteto extend a lower-level*testcore.TestEnvinstead oftestcore.FunctionalTestBase, and renameNexusTestBaseSuiteto a more aptly-namedNexusTestEnvparallelsuitewhich required edits of the following types:s.GetTestCluster()toenv.GetTestCluster(), because now these things are provided by theNexusTestEnvenvironment rather than the suite as beforetestcore.WithDedicatedCluster()for isolation, otherwise tests usingcaptureto assert on metrics would get polluteds.Eventually(...)(or some others.Assertion(...)type calls) -->require.Eventually(t, ...), so the failures are reported at the right test rather than at suite level. Previously tests were not ran in parallel so this may not have been an issues.GetTestCluster().FrontendClient()...-->env.FrontendClient()...Why?
Migrate to the latest and greatest, and make clearer the division between what's provided by the suite vs. env. This PR also enables parallel runs of tests, which sped up test runtime (see bottom of PR for rudimentary benchmarks).
How did you test it?
Test runtimes
nexus_api_test.go
Before:
After:
nexus_api_validation_test.go
Before:
After:
nexus_workflow_test.go
Before:
After: