Skip to content

examples: add node & cross-browser example#1502

Merged
laurentsenta merged 57 commits intotestground:masterfrom
GlenDC:feat/1386-browser-hello-advanced-test
Dec 5, 2022
Merged

examples: add node & cross-browser example#1502
laurentsenta merged 57 commits intotestground:masterfrom
GlenDC:feat/1386-browser-hello-advanced-test

Conversation

@GlenDC
Copy link
Copy Markdown
Contributor

@GlenDC GlenDC commented Oct 24, 2022

Add advanced browser example testplan Goal of this plan is to show how you can define a testplan which can be run in a node environment as well as within a browser, without having to change anything to your testplan code.

This does mean we also need to allow the @testground/sdk javascript package to be run in the browser as-is. To allow this I have a branch which I run this testplan against at https://github.com/glendc/sdk-js/tree/feat/support-browser:

  • the FS features are not possible in the browser, so these need to be disabled;
  • the environment variables are not available when creating the runtime from env in browser,
    so for the browser we instead assume that these are available in the window object as window.testground.env;

For both these changes we assume one is in the browser when process.title === 'browser', this because the browser npm (polyfill webpack) module sets that as the process global variable.

Within the example the testplan is found under src/, the other three directories form together the "magic" solution to allow the node testplan to also be run in the browser:

  • scripts/: contains a polyfill folder to inject an import in src/index.js which is required to polyfill some logic required by winston (an import by @testground/sdk)
  • browser/: the entry point for the docker:generic testplan, and which will spawn and control everything else;
    • browser/server/: simple express served — controlled by the browser/ and which will serve the web-pack'd testplan;

TODO:

  • allow debug breakpoints in chrome://inspect without having to refresh; (not an issue for me, and don't think there is a way to do it otherwise)
  • make the tests actually run and pass (as for now we no longer have errors and everything seems polyfill'ed, but the test doesn't seem to run (despite the fact that we do see logs coming through);
  • open a @testground/sdk MR and align on the approach there: my goal is that this sdk can be used just the same from the browser as well as the node env;
  • make the browser magic solution more smart in terms of how to wait for test to finish: somehow hook into an event which will be called when testplan is finished, not before and not later;

Related PRs:

Potential improvements but probably out of scope:

  • it would be great if we can choose what exposed port we bind to on the host machine (instead of currently any available open port) as it would mean we can always inspect against the same chrome address (but given this is defined by the docker Go library I do not have much hope for this one);

In function of #1386.

glendc added 28 commits October 9, 2022 22:57
goal will be to be able to run that same test
both in node as well as browser
webpack is used to bundle the testplan to be served from
the actual playwright-driven webpage
to allow for remote debugging
failing to get proper port exposed...
to allow in worst case to see all browser context up to that point
- use fork of sdk-js to support browser
- prepare window env up front
@GlenDC GlenDC changed the title Draft: advanced browser example testplan Draft: 1386: advanced browser example testplan Oct 24, 2022
@laurentsenta laurentsenta changed the title Node/cross- browser example dual testplan examples: add node & cross-browser example Nov 14, 2022
Copy link
Copy Markdown
Contributor

@laurentsenta laurentsenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there, thanks for sharing! These are the 3 fixes I'd like to solve before merging (nothing big):

cc @galargh and @nonsense if they want to review too.

@GlenDC
Copy link
Copy Markdown
Contributor Author

GlenDC commented Nov 15, 2022

First step is done. Third I cannot do yet until merged (if you want to block on that).
For step 2 I'll require some help as I'm not sure how I can pass these build args while using testground build I suppose it is build-cfg or something like that, but not sure how to use it.

@GlenDC GlenDC force-pushed the feat/1386-browser-hello-advanced-test branch from 76ee2d8 to bfaa5b3 Compare November 18, 2022 03:07
@GlenDC GlenDC requested review from laurentsenta and removed request for galargh and nonsense November 29, 2022 02:42
@GlenDC
Copy link
Copy Markdown
Contributor Author

GlenDC commented Nov 29, 2022

All tests pass @laurentsenta and last comments have been applied AFAIK.
Please give it another review and merge if possible.

Copy link
Copy Markdown
Contributor

@laurentsenta laurentsenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for sharing; it's pretty exciting to see this used soon!

I added @galargh and @achingbrain to give them a chance to review as well,

A few follow-up tasks, let's not block on this:

  • run the sync test also, ideally a single test that connects all the browsers + node.
  • move the build process to a single builder (docker:generic I guess), so that the runtime is actually "just" a flag.
  • make a release for the js-sdk and tag a version.

@laurentsenta laurentsenta merged commit db66d28 into testground:master Dec 5, 2022
laurentsenta pushed a commit that referenced this pull request Dec 8, 2022
Implement follow-ups from: #1502 (review).

Summary:

- replace the `BrowserKind` test paramater with `runtime`, which by default is `node`, but
   - can also be set to `chromium`, `firefox` and `webkit`.
 - enable the sync test for the browser-node example
    - also added a sh script to run that same test for the regular `js` example (while I was at it)
- update sdk-js used

Contributes to #1386
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants