From d9c1d287c8c0352773d67bed5eb58d0c8f5d6585 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Fri, 24 Nov 2023 09:52:17 +0000 Subject: [PATCH] aspect workflows: add initial aspect workflow yaml (#56569) * add initial aspect workflow yaml - try reading docker config env var - bump both timers - bump grpc test timout to long - skip additional perforce test and run all tests - bump timeouts - more timeout bumps and skip p4 test - bump doc:test timeout - bump e2e_test timeout - bump database/connections/live timeout - tag integration tests as exclusive * add recommended bazelrc in workflows to speed up cold builds * disable experimental_fetch_all_coverage_outputs * port changes from https://github.com/sourcegraph/sourcegraph/compare/aspect-trial/wb-add-initial-config...aspect-trial/wb-add-initial-config-greg * bazel configure * add //:postcss_config_js as data target to client/web * remove postcss added in debug * use node-fetch and only test codeintellify * use testing fetch.js setup * fix syntax in testSetup * various fixes revert timeout bump on repository test re-enable git p4 test add testing from shared deps bazel configure * update comments on skipped tests * restore `is_percy_enabled` for mocha_test * slightly increase repo cloning wait * use process.cwd instead of __dirname * set sizing to moderate as well for embeddings * remove setting CI in workflows yaml * fix sizing * workflow yaml tweaks and bazelrc tweaks * make bazelrc consistent with what was in workflow yaml --------- Co-authored-by: Jean-Hadrien Chabran Co-authored-by: Greg Magolan --- .aspect/bazelrc/ci.sourcegraph.bazelrc | 15 ++++++----- .aspect/workflows/bazelrc | 3 +++ .aspect/workflows/config.yaml | 26 +++++++++++++++++++ client/codeintellify/vitest.config.ts | 3 +-- client/web/BUILD.bazel | 1 + client/web/src/end-to-end/BUILD.bazel | 4 ++- .../frontend-platform/site-admin.test.ts | 4 ++- .../frontend-platform/theme-switcher.test.ts | 4 ++- client/web/src/integration/notebook.test.ts | 2 +- cmd/blobstore/internal/blobstore/BUILD.bazel | 2 +- cmd/embeddings/shared/BUILD.bazel | 1 + cmd/frontend/graphqlbackend/BUILD.bazel | 2 +- .../internal/authz/resolvers/BUILD.bazel | 2 +- doc/BUILD.bazel | 2 +- .../database/connections/live/BUILD.bazel | 2 +- internal/gqltestutil/repository.go | 2 +- internal/httpcli/client_test.go | 6 ++--- testing/BUILD.bazel | 1 + 18 files changed, 60 insertions(+), 22 deletions(-) create mode 100644 .aspect/workflows/bazelrc create mode 100644 .aspect/workflows/config.yaml diff --git a/.aspect/bazelrc/ci.sourcegraph.bazelrc b/.aspect/bazelrc/ci.sourcegraph.bazelrc index 572c553af49..fa918f21f0a 100644 --- a/.aspect/bazelrc/ci.sourcegraph.bazelrc +++ b/.aspect/bazelrc/ci.sourcegraph.bazelrc @@ -12,9 +12,10 @@ build --repository_cache=/home/buildkite/repocache-sourcegraph build --test_env=PATH # Needed for DB in CI -build --test_env=PGUSER -build --test_env=PGSSLMODE -build --test_env=PGDATABASE +test --test_env=PGUSER=postgres +test --test_env=PGPASSWORD=postgres +test --test_env=PGSSLMODE=disable +test --test_env=PGDATABASE=postgres # Allow tests to understand they're running in CI, which forces dbtest to drop database even in case of failures. # TODO(JH) we should instead wipe all templates after a job finishes. @@ -30,9 +31,11 @@ build --test_env=BUILDKITE # Needed for mocha tests # We have to use the `--define` flag here instead of `--test_env` because # the mocha tests target is the build target and it's tested with `build_test`. -build --define=E2E_HEADLESS=false -build --define=E2E_SOURCEGRAPH_BASE_URL="http://localhost:7080" -build --define=DISPLAY=:99 +test --define=E2E_HEADLESS=false +# if we set this to localhost, chrome will refuse to conenct since local host is in its HTTP Strict Transport Security +# by setting the loopback address we get passed that +test --define=E2E_SOURCEGRAPH_BASE_URL="http://127.0.0.1:7080" +test --define=DISPLAY=:99 # Provides git commit, branch information to build targets like Percy via status file. # https://bazel.build/docs/user-manual#workspace-status diff --git a/.aspect/workflows/bazelrc b/.aspect/workflows/bazelrc new file mode 100644 index 00000000000..2b7d0e2b670 --- /dev/null +++ b/.aspect/workflows/bazelrc @@ -0,0 +1,3 @@ +common --remote_download_minimal +common --nobuild_runfile_links +common --noexperimental_reuse_sandbox_directories diff --git a/.aspect/workflows/config.yaml b/.aspect/workflows/config.yaml new file mode 100644 index 00000000000..95725c7650e --- /dev/null +++ b/.aspect/workflows/config.yaml @@ -0,0 +1,26 @@ +--- +bazel: + rcfiles: + - ".aspect/bazelrc/ci.sourcegraph.bazelrc" + flags: + # This flag is required because otherwise the integration tests fail with `fcmod` Operation not permitted + # which is probably related to the launced containers writing to mapped in directories as root and then + # when the container exits the files that are left over are root. + # TODO(burmudar): launch containers with uid/guid mapped in + - --noexperimental_reuse_sandbox_directories +env: + REDIS_CACHE_ENDPOINT: ":6379" + GIT_PAGER: '' +tasks: + # Checks that BUILD file content is up-to-date with sources + # gazelle: + # Checks that all tests are passing + test: + include_eternal_tests: true + targets: + - //... + - //testing:grpc_backend_integration_test + # This target should only really run when on main which we aren't handling. For the time being while we + # evaluate Aspect Workflows it is ok + # TODO(burmudar): Let this only run on main branch + - //testing:codeintel_integration_test diff --git a/client/codeintellify/vitest.config.ts b/client/codeintellify/vitest.config.ts index e16b5fdd7c1..0a2927f06bd 100644 --- a/client/codeintellify/vitest.config.ts +++ b/client/codeintellify/vitest.config.ts @@ -3,7 +3,6 @@ import { defineProjectWithDefaults } from '../../vitest.shared' export default defineProjectWithDefaults(__dirname, { test: { environment: 'jsdom', - setupFiles: ['src/testSetup.test.ts'], - singleThread: true, // got `failed to terminate worker` occasionally in Bazel CI + setupFiles: ['src/testSetup.test.ts', '../testing/src/fetch.js'], }, }) diff --git a/client/web/BUILD.bazel b/client/web/BUILD.bazel index 7d8d2f93379..8b79f23430c 100644 --- a/client/web/BUILD.bazel +++ b/client/web/BUILD.bazel @@ -2120,6 +2120,7 @@ esbuild( minify = True, sourcemap = "linked", splitting = True, + tags = ["exclusive"], visibility = ["//client/web/dist:__subpackages__"], deps = ESBUILD_CONFIG_DEPS, ) diff --git a/client/web/src/end-to-end/BUILD.bazel b/client/web/src/end-to-end/BUILD.bazel index b612815af69..55799275f1b 100644 --- a/client/web/src/end-to-end/BUILD.bazel +++ b/client/web/src/end-to-end/BUILD.bazel @@ -58,7 +58,9 @@ ts_project( mocha_test( name = "e2e", timeout = "moderate", - env = {"INCLUDE_ADMIN_ONBOARDING": "false"}, + env = { + "INCLUDE_ADMIN_ONBOARDING": "false", + }, node_toolchain = select({ "//:darwin_docker_e2e_go": "@nodejs_darwin_arm64//:node_toolchain", "//conditions:default": None, diff --git a/client/web/src/end-to-end/frontend-platform/site-admin.test.ts b/client/web/src/end-to-end/frontend-platform/site-admin.test.ts index db4a9565b5e..fe77b7e8ae9 100644 --- a/client/web/src/end-to-end/frontend-platform/site-admin.test.ts +++ b/client/web/src/end-to-end/frontend-platform/site-admin.test.ts @@ -9,7 +9,9 @@ import { initEndToEndTest } from '../utils/initEndToEndTest' const { sourcegraphBaseUrl } = getConfig('gitHubDotComToken', 'sourcegraphBaseUrl') -describe('Site Admin', () => { +// Since the test inside the describe is skipped the after does not execute. Consequently the page does not get closed +// causing subsequent failures. This is a known bug in Mocha +describe.skip('Site Admin', () => { let driver: Driver before(async function () { diff --git a/client/web/src/end-to-end/frontend-platform/theme-switcher.test.ts b/client/web/src/end-to-end/frontend-platform/theme-switcher.test.ts index 8dece4ab28c..e67ffc303f6 100644 --- a/client/web/src/end-to-end/frontend-platform/theme-switcher.test.ts +++ b/client/web/src/end-to-end/frontend-platform/theme-switcher.test.ts @@ -9,7 +9,9 @@ import { initEndToEndTest } from '../utils/initEndToEndTest' const { sourcegraphBaseUrl } = getConfig('gitHubDotComToken', 'sourcegraphBaseUrl') -describe('Theme switcher', () => { +// Since the test inside the describe is skipped the after does not execute. Consequently the page does not get closed +// causing subsequent failures. This is a known bug in Mocha +describe.skip('Theme switcher', () => { let driver: Driver before(async () => { diff --git a/client/web/src/integration/notebook.test.ts b/client/web/src/integration/notebook.test.ts index bf1f1972622..f2ab377aff5 100644 --- a/client/web/src/integration/notebook.test.ts +++ b/client/web/src/integration/notebook.test.ts @@ -73,7 +73,7 @@ const viewerSettings: Partial = } const now = new Date() -const downloadPath = process.env.TEST_TMPDIR || __dirname +const downloadPath = process.env.TEST_TMPDIR || process.cwd() const notebookFixture = (id: string, title: string, blocks: NotebookFields['blocks']): NotebookFields => ({ __typename: 'Notebook', diff --git a/cmd/blobstore/internal/blobstore/BUILD.bazel b/cmd/blobstore/internal/blobstore/BUILD.bazel index 66c803d7f8f..2f2975c5831 100644 --- a/cmd/blobstore/internal/blobstore/BUILD.bazel +++ b/cmd/blobstore/internal/blobstore/BUILD.bazel @@ -25,7 +25,7 @@ go_library( go_test( name = "blobstore_test", - timeout = "short", + timeout = "moderate", srcs = ["blobstore_test.go"], data = glob(["testdata/**"]), deps = [ diff --git a/cmd/embeddings/shared/BUILD.bazel b/cmd/embeddings/shared/BUILD.bazel index 460cd9154cc..40668b409a0 100644 --- a/cmd/embeddings/shared/BUILD.bazel +++ b/cmd/embeddings/shared/BUILD.bazel @@ -69,6 +69,7 @@ copy_file( # gazelle:exclude testdata go_test( name = "shared_test", + size = "medium", timeout = "moderate", srcs = [ "context_qa_test.go", diff --git a/cmd/frontend/graphqlbackend/BUILD.bazel b/cmd/frontend/graphqlbackend/BUILD.bazel index 99952c4223c..b654ba44f58 100644 --- a/cmd/frontend/graphqlbackend/BUILD.bazel +++ b/cmd/frontend/graphqlbackend/BUILD.bazel @@ -386,7 +386,7 @@ go_library( go_test( name = "graphqlbackend_test", - timeout = "moderate", + timeout = "long", srcs = [ "access_requests_test.go", "access_tokens_test.go", diff --git a/cmd/frontend/internal/authz/resolvers/BUILD.bazel b/cmd/frontend/internal/authz/resolvers/BUILD.bazel index a12c5b7146e..de97ae485bb 100644 --- a/cmd/frontend/internal/authz/resolvers/BUILD.bazel +++ b/cmd/frontend/internal/authz/resolvers/BUILD.bazel @@ -44,7 +44,7 @@ go_library( go_test( name = "resolvers_test", - timeout = "short", + timeout = "moderate", srcs = [ "permissions_sync_jobs_test.go", "resolver_test.go", diff --git a/doc/BUILD.bazel b/doc/BUILD.bazel index 527bfdff3e0..8e5533eb447 100644 --- a/doc/BUILD.bazel +++ b/doc/BUILD.bazel @@ -1,7 +1,7 @@ sh_test( name = "test", size = "small", - timeout = "moderate", + timeout = "long", srcs = ["test.sh"], args = ["$(location //dev/tools:docsite)"], data = [ diff --git a/internal/database/connections/live/BUILD.bazel b/internal/database/connections/live/BUILD.bazel index 0976a6ad9ff..cd397a24b1a 100644 --- a/internal/database/connections/live/BUILD.bazel +++ b/internal/database/connections/live/BUILD.bazel @@ -26,7 +26,7 @@ go_library( go_test( name = "live_test", - timeout = "moderate", + timeout = "long", srcs = ["migration_test.go"], embed = [":live"], tags = [ diff --git a/internal/gqltestutil/repository.go b/internal/gqltestutil/repository.go index 4735c77e4cc..0e8524ba698 100644 --- a/internal/gqltestutil/repository.go +++ b/internal/gqltestutil/repository.go @@ -14,7 +14,7 @@ import ( // // This method requires the authenticated user to be a site admin. func (c *Client) WaitForReposToBeCloned(repos ...string) error { - timeout := 120 * time.Second + timeout := 130 * time.Second return c.WaitForReposToBeClonedWithin(timeout, repos...) } diff --git a/internal/httpcli/client_test.go b/internal/httpcli/client_test.go index ca1498aedf4..ddaaf888a1c 100644 --- a/internal/httpcli/client_test.go +++ b/internal/httpcli/client_test.go @@ -13,7 +13,6 @@ import ( "net" "net/http" "net/http/httptest" - "os" "reflect" "strings" "sync/atomic" @@ -406,9 +405,8 @@ func TestErrorResilience(t *testing.T) { // hardcode what go returns for DNS not found to avoid // flakiness across machines. However, CI correctly respects // this so we continue to run against a real DNS server on CI. - if os.Getenv("CI") == "" { - cli.Transport = notFoundTransport{} - } + // TODO(burmudar): Fix DNS infrastructure in Aspect Workflows Infra + cli.Transport = notFoundTransport{} return nil }, NewErrorResilientTransportOpt( diff --git a/testing/BUILD.bazel b/testing/BUILD.bazel index ca9851e3f9b..a87879f356f 100644 --- a/testing/BUILD.bazel +++ b/testing/BUILD.bazel @@ -2,6 +2,7 @@ load(":defs.bzl", "server_integration_test") server_integration_test( name = "e2e_test", + timeout = "long", args = [ "$(location //client/web/src/end-to-end:e2e)", "$(rootpath //:mocha_config)",