Conversation
mavemuri
changed the title
| ) | ||
|
|
||
| // QueryWorkflowTarget identifies the workflow that the "query-workflow" Nexus operation queries. | ||
| // Only WorkflowID is required |
There was a problem hiding this comment.
nit
| // Only WorkflowID is required | |
| // WorkflowID is required. |
| historyMatcher HistoryMatcher | ||
| expectedUnsupportedErrs map[clioptions.Language]string | ||
| expectedWorkflowError string | ||
| populateFn func(t *testing.T) testCase |
There was a problem hiding this comment.
Hm, I don't love it. I think maybe this was the wrong direction; instead we should isolate the SDK runs by using separate namespaces. I noodled a bit one it and came up with this: #462
There was a problem hiding this comment.
Agree, per-SDK ns is cleaner - thanks for raising the pr
| for i, yv := range yList { | ||
| if !looselyEqual(x[i], yv) { | ||
| return false | ||
| } |
There was a problem hiding this comment.
That's fine with me; can we tighten the comment there?
// Match the unordered expected list so a spec can
// assert only what it knows. Eg, when asserting links, if runID is
// not known, it should be possible to assert on just WID+NS
I don't quite understand what it's saying tbh. Esp. since they are not "unordered".
I'd omit it or add a shorter comment maybe on if !looselyEqual(x[i], yv) { to explain that this recurses into list elements (if that's what it means to say). Or drop the comment.
2 participants
What was changed
Added tests for Query as a Nexus Op
Why?
part of the SDK ergonomics efforts
Checklist
Closes NEXUS-692
How was this tested: tested locally
Any docs updates needed? No