Add test harness for headless browser testing #881

sethvargo · 2020-10-22T00:57:04Z

After evaluation, I decided to use chromedp since it's a pure-go solution that should work headlessly on our existing CI infrastructure (fingers crossed).

This also adds a few new internal-only packages to help with testing:

internal/project - project-wide helpers. Currently the only function is one that returns the filesystem root of the project, which is needed for telling the server where to find its assets. It will also be used by enx-redirector and any other filesystem-ey things.
internal/envstest - globally shared test helpers. I'm going to start centralizing things here across all our test suites. This package also includes all the logic and helpers for spinning up a full in-memory UI server (with database and dependencies) for local testing.
internal/browser - wrapper around chromedp. It's small, but I expect to add more helpers and things as we expand our testing.

Release Note

Add test harness for headless browser testing

sethvargo · 2020-10-22T01:01:29Z

Hmm... I guess chromedp needs Chrome 😄

@chaodaiG any ideas of how we'd get chrome into our binary? There's a base image we could borrow from: https://github.com/chromedp/docker-headless-shell

mikehelmick · 2020-10-22T01:07:51Z

For FB config - can we make that injected config from the runtime environment?

jeremyfaller

Modulo some nits, this LGTM.

internal/browser/executor.go

jeremyfaller · 2020-10-22T13:53:58Z

internal/envstest/server.go

+// NewServer creates a new test UI server instance. When this function returns,
+// a full UI server will be running locally on a random port. Cleanup is handled
+// automatically.
+func NewServer(tb testing.TB) *TestServerResponse {


Small nomenclature niggle. The function's entitled NewServer yet it doesn't return a Server type, it returns a response. It feels like here (and the below function NewServerConfig) are either misnamed, or the the type returned is.

It returns the server and everything needed to run it. We have to return things like the database because tests might need to insert or assert records.

I'm open to other names

internal/envstest/server.go

jeremyfaller · 2020-10-22T14:02:35Z

internal/project/root.go

+var _, self, _, _ = runtime.Caller(0)
+
+// Root returns the filepath to the root of this project.
+func Root() string {


This could be a global constant, like self, and the init generation code in the compiler will sort out the dependency chain.

It'd be a global var, which would be mutable. AFAIK, you can't create a constant that uses filepath.Join since it wouldn't be constant across systems.

fair point. Root could still be a LHS.

What's an LHS?

left hand side. common compiler nomenclature for a "result of an expression"

internal/browser/executor.go

jeremyfaller · 2020-10-22T14:07:55Z

internal/envstest/random.go

+}
+
+// RandomString returns a random hex-encoded string of the given length.
+func RandomString(tb testing.TB, length int) string {


API is slightly clunky. length is the length of the random string, not the encoded string. Might want to document.

It is the length of the encoded string (that's why I divide by 2).

pkg/controller/home/home_test.go

pkg/database/database_util.go

chaodaiG · 2020-10-22T14:25:47Z

Hmm... I guess chromedp needs Chrome 😄

@chaodaiG any ideas of how we'd get chrome into our binary? There's a base image we could borrow from: https://github.com/chromedp/docker-headless-shell

How about add docker-headless-shell in prow test base image, and chromedp references it's entrypoint /headless-shell/headless-shell as ExecPath https://godoc.org/github.com/knq/chromedp#ExecPath?

sethvargo · 2020-10-22T15:13:55Z

@chaodaiG That could work, but it will make running tests locally harder. It would be better if we could have chrome or chrome-headless in $PATH.

chaodaiG · 2020-10-22T15:23:49Z

@chaodaiG That could work, but it will make running tests locally harder. It would be better if we could have chrome or chrome-headless in $PATH.

Agreed, we can append /headless-shell to PATH in dockerfile

sethvargo · 2020-10-22T15:33:01Z

@chaodaiG sounds good. Do you have any cycles to work on that today? I imagine we're going to have some trial and error to get it working against this PR.

chaodaiG · 2020-10-22T16:34:28Z

@chaodaiG sounds good. Do you have any cycles to work on that today? I imagine we're going to have some trial and error to get it working against this PR.

google/exposure-notifications-server#1104
@sethvargo could you take a quick look? If there is no obvious problem I'll push the image for testing current PR

sethvargo · 2020-10-22T16:55:40Z

Just approved

chaodaiG · 2020-10-22T17:09:26Z

Image pushed

chaodaiG · 2020-10-22T17:11:59Z

/retest

sethvargo · 2020-10-22T17:47:59Z

Hmm @chaodaiG

could not dial "ws://127.0.0.1:42897/devtools/browser/91b93c42-b53a-44d1-8b9c-7c822949108a": EOF

Different error this time. Do we need to do something to allow for that networking?

sethvargo

@chaodaiG @jeremyfaller @whaught this is ready for review now

sethvargo · 2020-10-29T15:04:26Z

internal/project/root.go

+var _, self, _, _ = runtime.Caller(0)
+
+// Root returns the filepath to the root of this project.
+func Root() string {


What's an LHS?

chaodaiG

LGTM generally, left a comment

internal/project/root.go

After evaluation, I decided to use chromedp since it's a pure-go solution that should work headlessly on our existing CI infrastructure (fingers crossed). This also adds a few new internal-only packages to help with testing: - `internal/project` - project-wide helpers. Currently the only function is one that returns the filesystem root of the project, which is needed for telling the server where to find its assets. It will also be used by enx-redirector and any other filesystem-ey things. - `internal/envstest` - globally shared test helpers. I'm going to start centralizing things here across all our test suites. This package also includes all the logic and helpers for spinning up a full in-memory UI server (with database and dependencies) for local testing. - `internal/browser` - wrapper around chromedp. It's small, but I expect to add more helpers and things as we expand our testing.

chaodaiG · 2020-10-29T15:40:49Z

/lgtm
/approve

google-oss-robot · 2020-10-29T15:40:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chaodaiG, sethvargo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [chaodaiG,sethvargo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

google-oss-robot requested review from jeremyfaller and mikehelmick October 22, 2020 00:57

googlebot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Oct 22, 2020

google-oss-robot added approved labels Oct 22, 2020

jeremyfaller reviewed Oct 22, 2020

View reviewed changes

sethvargo force-pushed the sethvargo/browser branch 2 times, most recently from 1d683f6 to a56da70 Compare October 28, 2020 00:37

google-oss-robot added size/XXL and removed size/XL labels Oct 28, 2020

sethvargo force-pushed the sethvargo/browser branch 9 times, most recently from 3f52773 to f68dea8 Compare October 28, 2020 22:45

sethvargo force-pushed the sethvargo/browser branch 3 times, most recently from e154154 to b1fb8e6 Compare October 29, 2020 15:05

sethvargo commented Oct 29, 2020

View reviewed changes

chaodaiG approved these changes Oct 29, 2020

View reviewed changes

internal/project/root.go Show resolved Hide resolved

google-oss-robot assigned chaodaiG Oct 29, 2020

google-oss-robot added the lgtm label Oct 29, 2020

sethvargo force-pushed the sethvargo/browser branch from b1fb8e6 to 1248464 Compare October 29, 2020 15:34

google-oss-robot removed the lgtm label Oct 29, 2020

google-oss-robot added the lgtm label Oct 29, 2020

google-oss-robot merged commit 73782ae into main Oct 29, 2020

google-oss-robot deleted the sethvargo/browser branch October 29, 2020 15:41

github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2020

Add test harness for headless browser testing #881

Add test harness for headless browser testing #881

Uh oh!

Conversation

sethvargo commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sethvargo commented Oct 22, 2020

Uh oh!

mikehelmick commented Oct 22, 2020

Uh oh!

jeremyfaller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremyfaller Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chaodaiG commented Oct 22, 2020

Uh oh!

sethvargo commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chaodaiG commented Oct 22, 2020

Uh oh!

sethvargo commented Oct 22, 2020

Uh oh!

chaodaiG commented Oct 22, 2020

Uh oh!

sethvargo commented Oct 22, 2020

Uh oh!

chaodaiG commented Oct 22, 2020

Uh oh!

chaodaiG commented Oct 22, 2020

Uh oh!

sethvargo commented Oct 22, 2020

Uh oh!

sethvargo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaodaiG left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chaodaiG commented Oct 29, 2020

Uh oh!

google-oss-robot commented Oct 29, 2020

Uh oh!

Uh oh!

sethvargo commented Oct 22, 2020 •

edited

Loading

jeremyfaller Oct 29, 2020 •

edited

Loading

sethvargo commented Oct 22, 2020 •

edited

Loading