Multi-platform CI #1293
Multi-platform CI #1293
Conversation
cretz
force-pushed
the
multi-ci
branch
11 times, most recently
from
7c2f1ed to
911d83e
Compare
| @@ -169,6 +169,7 @@ func isFileAutogenerated(_ string) bool { | |||
| } | |||
|
|
|||
| func mustProcessPath(path string) bool { | |||
| path = filepath.ToSlash(path) | |||
There was a problem hiding this comment.
This was not Windows-safe before
| @@ -71,7 +71,6 @@ func TestGRPCInterceptor(t *testing.T) { | |||
| require.Len(t, timers, 1) | |||
| require.Equal(t, metrics.TemporalRequestLatency+"_my_suffix", timers[0].Name) | |||
| require.Equal(t, map[string]string{metrics.OperationTagName: "Check"}, timers[0].Tags) | |||
| require.Greater(t, timers[0].Value(), 0*time.Second) | |||
There was a problem hiding this comment.
On some platforms, this would actually appear to the metric collector as 0 seconds in flaky/fast cases
| require.Never(t, tryWrite(completionChans[1]), 2*time.Second, 100*time.Millisecond, | ||
| "Should be no reader on the task1's completion channel as task0 holds the ctx lock") |
There was a problem hiding this comment.
The way that this poor require.Never is written, the callback can technically be invoked even after this is returned, which was causing race conditions with the closing of the channel later. Decided this part of the test isn't really needed to test the rest.
| // TODO(cretz): There is a bug with search attribute names on standard | ||
| // visibility with eager workflow start | ||
| options.EnableEagerStart = false |
There was a problem hiding this comment.
| if err != nil { | ||
| if err = ts.registerNamespace(); err != nil { | ||
| return err | ||
| } else if err = ts.ensureSearchAttributes(); err != nil { |
There was a problem hiding this comment.
For dev server, the CustomKeywordField search attribute isn't present like it is in docker compose
# Conflicts: # Makefile
| @@ -2685,6 +2686,9 @@ func (ts *IntegrationTestSuite) TestDeterminismUpsertSearchAttributesConditional | |||
| options.SearchAttributes = map[string]interface{}{ | |||
| "CustomKeywordField": "unset", | |||
| } | |||
| // TODO(cretz): There is a bug with search attribute names on standard | |||
There was a problem hiding this comment.
Can we replace with the issue link
|
I have to go mod tidy some stuff and maybe fix other things w/ remove-gogo merge before CI passes, but won't merge w/out re-review if I have to alter anything serious. |
| // Find the root directory from this directory | ||
| _, thisFile, _, _ := runtime.Caller(0) | ||
| b.thisDir = filepath.Join(thisFile, "..") | ||
| b.rootDir = filepath.Join(b.thisDir, "../../../") |
There was a problem hiding this comment.
You could call into git rev-parse --show-toplevel instead. We know it's a git repo
There was a problem hiding this comment.
I think this code is much clearer than making a git call. It's really easy to just get a relative directory of a current file.
cretz
force-pushed
the
multi-ci
branch
2 times, most recently
from
a72d146 to
e276951
Compare
What was changed
When ready to merge, will change the required build targets in GH branch protection
Checklist