ui: reintroduce end-to-end UI tests with cypress #85095

sjbarag · 2022-07-26T21:41:46Z

A recent commit [1] removed the stale, unused end-to-end tests. In their
place, add newly-written tests that can run headlessly in TeamCity (via
Docker) as a quick-failing health check and as a more exhaustive suite.

[1] 3d171b2 (ui: remove unused end-to-end UI tests, 2022-07-25)

Release note: None

cockroach-teamcity · 2022-07-26T21:41:53Z

This change is

sjbarag · 2022-07-26T21:42:06Z

do-not-merge added because I need to merge cockroachdb/yarn-vendored#98 first :)

sjbarag · 2022-07-26T21:45:49Z

Don't get scared by the line count here — most of that is a new yarn.lock!

sjbarag · 2022-07-26T21:48:25Z

I guess I should add a README.md to that tree huh 😅

laurenbarker

I have some questions, but overall this Someone else should do a pass on the docker/bazel stuff who has relevant knowledge.

Reviewed 25 of 26 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @sjbarag and @srosenberg)

build/teamcity/cockroach/ci/tests/ui_e2e_test_impl.sh line 3 at r2 (raw file):

#!/usr/bin/env bash
function build_docker_image() {
  # Buffer noisy output and only print it on failure.

Smart! Do we do this in other places?

build/teamcity/cockroach/ci/tests/ui_e2e_test_impl.sh line 15 at r2 (raw file):

  SPEC_ARG=""
  if [ "health" = "${$1:='EMPTY'}" ]; then
    SPEC_ARG="--spec 'cypress/e2e/health-check/**'"

Is there a reason to specify the directory? Does --e2e not work because the tests are nested in the health-check directory?

pkg/ui/workspaces/e2e-tests/.eslintrc.json line 13 at r2 (raw file):

    "@typescript-eslint/no-inferrable-types": "off",
    "@typescript-eslint/ban-types": "off",
    "@typescript-eslint/no-unused-vars": "off",

Why are these rules overridden to off? Wouldn't we want these on?

pkg/ui/workspaces/e2e-tests/Dockerfile line 28 at r2 (raw file):

#     cockroachdb/cockroach-ci-ui

FROM cypress/browsers:node16.14.2-slim-chrome100-ff99-edge

Ah, maybe we should use this image in managed-service too, so the cypress version in CI always matches what we use locally 🤔

pkg/ui/workspaces/e2e-tests/Dockerfile line 43 at r2 (raw file):

# Use the compiled CRDB binary from a previous TC job via Artifact Dependency:
# TODO(barag): include a TC link here

Is this a TODO for post-merge?

pkg/ui/workspaces/e2e-tests/prettier.config.js line 12 at r2 (raw file):

module.exports = {
  trailingComma: "all",

Our shared eslint config includes prettier rules. Do we need this config?

pkg/ui/workspaces/e2e-tests/cypress/fixtures/example.json line 2 at r2 (raw file):

{
  "name": "Using fixtures to represent data",

Are these being used? It looks like dummy data.

pkg/ui/workspaces/e2e-tests/cypress/plugins/index.ts line 29 at r2 (raw file):

The plugins file is no longer supported as of Cypress version 10.0.0.
https://docs.cypress.io/guides/references/migration-guide#Plugins-File-Removed

Was this file added by default? It's no longer supported 🤔

pkg/ui/workspaces/e2e-tests/cypress/support/commands.ts line 33 at r2 (raw file):

});

import "@testing-library/cypress/add-commands";

Why is this at the bottom?

pkg/ui/workspaces/e2e-tests/cypress/support/index.ts line 41 at r2 (raw file):

}

import "./commands";

Should the imports be at the bottom?

sjbarag

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @laurenbarker, @sjbarag, and @srosenberg)

build/teamcity/cockroach/ci/tests/ui_e2e_test_impl.sh line 3 at r2 (raw file):