Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create test to browser compatibility #284

Open
luizstacio opened this issue May 18, 2022 · 20 comments
Open

Create test to browser compatibility #284

luizstacio opened this issue May 18, 2022 · 20 comments
Assignees
Labels

Comments

@luizstacio
Copy link
Member

luizstacio commented May 18, 2022

Fuels-ts would probably be used on many WebApps and this requires adding tests to run jest on a browser environment.

See also:

@camsjams
Copy link
Contributor

camsjams commented Jan 4, 2023

This would catch bugs and implementation details like these:

@danielbate
Copy link
Contributor

danielbate commented Aug 30, 2023

Current work in progress report on browser compatibility testing:

So far I’ve tried a number of different approaches and tackled a few different challenges that can be summarised into the following categories, that gets us closer to cross environment (and potentially cross framework testing) within the TS SDK.

Test Grouping

The agreed upon approach is inspired by jest-runner-groups where we append test cases with a comment block that outlines the testing groups it is part of:

/**
  * @group node
  * @group browser
  */
describe("Testcase", () => {
...

Pretty much all testing frameworks that have been used allow some kind of regex matching, that allow us to pass in test files. @arboleya wrote a nice shell script that parses for test files for such group tags and returns file names, that can be used across frameworks.

We should keep away from custom test runners where possible to avoid compatibility issues between frameworks. Appending environments to test file names (my-test.browser.node.test.ts) was also trialled successfully for this reason (and to grep by file name), but doc tagging was preferred across the team.

Coverage merging

As we will now be running multiple test suites, we need to be able to merge test coverage to report confidently on the coverage across the repository. Most frameworks support Istanbul for coverage, and therefore a tool called NYC can be used to merge and report on coverage. @Dhaiwat10 implemented this in the initial implementation also with success.

We should hand off all coverage collection and reporting to Istanbul and thus NYC. Jest has it’s own Json format that made merging incompatible with NYC, I foresee this issue with other frameworks. Therefore NYC provides a succinct way across frameworks (so long as the framework supports NYC, I’m yet to test one that doesn’t.

The only caveat here is coverage reporting in our workflow actions. I recommend also utilising one that supports a test format such as LCOV rather than a framework (i.e. Jest) to ensure cross framework compatibility. Also would recommend avoiding JSON for cross framework reporting, due to differences in the schema across frameworks.

Testing frameworks and tools

So far I have tried a range of frameworks/tools to meet the following criteria:

  • Minimal interruption to our current test setup
  • A test syntax that works across frameworks / environments (potentially via an adapter)
  • Tools that are still under development and have good uptake and support from the community
  • Allows node and browser environment testing, in a real browser that gives direct access to browser APIs (some frameworks give access via some additional context)

The following frameworks/tools have been tried and I’ll summarise the outcomes:

Jest (standalone)

  • Our current framework, lots of support and ideal for node based testing
  • Needs modification and/or implementation of another framework and tool to implement browser testing

Jest (with Playwright)

  • Some good support for adapters to allow cross environment testing with Jest providing the test infrastructure
  • However, they are recommending moving to the official playwright runner following the implementation of playwright/test to provide a common syntax between Jest and Playwright
  • Main issue with this implementation was that Playwright runs tests in a browser like environment, rather than giving us direct access to the browser in tests (ref: https://playwright.dev/docs/evaluating) This means we do not have direct access to browser APIs and could throw up compatibility issues
  • This could be done in tandem with a JSDom testing environment to give a decent level of confidence but not the level of confidence we should aim to achieve with a complete implementation

Jest (with Puppeteer)

  • Similar to the above, Pupeeteer is the predecessor to Playwright
  • Main issue, as above, is that code is executed in a browser like environment

Jest (with Electon)

  • Came across this implementation when looking for a browser testing tool that would give direct access to the browser through Jest
  • Had success with the implementation and passed both node and browser tests, however required a decent amount of configuration, doesn’t seem to have much support and pretty hacky

Yet to try

  • Mocha and Vitest - both will require a complete reimplementation of our test infrastructure but I do want to investigate these as they seem to come with node and browser support out of the box
  • Jest w/ Cypress - I for-see this giving the same outcome as Playwright but will investigate
  • Playwright - does have unit testing functionality but need to see if it will fit all our requirements

Won’t try

  • Karma - deprecated although looked like a good option in the past
  • JSDom - could be used in tandem with a virtual browser but again doesn’t give us everything we are looking for

Conclusions (so far)

So far I have been prioritising augmentations of our current Jest infrastructure, as it is a well supported tool so framework adapters are pretty common, and to introduce minimal interruption to our current setup. However I am coming to believe that our new requirements may not be best fit for Jest. Hence I am going to continue investigating a complete change of test infrastructure (with benchmarks).

Successful grouping, merging and workflow reporting for the new requirements has been implemented in #1182 however the implemented tools and frameworks are subject to change

@arboleya
Copy link
Member

arboleya commented Aug 30, 2023

Thanks for getting to the bottom of this.

Jest doesn't offer a browser-runner, and each workaround opens up a can of worms.

I ran some experiments in my free time, and Vitest seems like the best solution to fulfill our needs.

Things like Mocha/Jasmine also offer browser-runners, but the required tooling around them is exhaustive.

I've been using these in conjunction with Instabul middlewares for a long time, and they haven't evolved much in 10 years.

@danielbate Let's talk and see where it leads us.

@danielbate
Copy link
Contributor

Pausing the Vitest investigations for now.

Currently impressions are that it is very fast (will benchmark once the full suite runs) and requires minimal configuration compared to Jest, but there appears to be a couple blockers with itself and our current setup. However positive as it seems to giving the highest level of confidence regarding browser compatibility testing.

  • Vite will automatically pre-bundle dependencies as native ESM, even if a package is only shipped as CJS or UMD. This is cool that this doesn't require any additional configuration unlike Jest, however I suspect this is why I am currently seeing discrepancies between node and browser tests. I would like to upgrade some of our packages that are shipped only in CJS. This is not only beneficial to this issue, but also to keep our packages up to date. Lodash and Ethers v5 are culprits to this during my investigation.
  • Cyclic deps in browser mode, I also believe this is related to the above - or potentially some additional config is required due to our mono-repo setup
  • Node dependencies need to be poly-filled or installed/served by source code
  • And a more firm blocker for our CI, headless mode is not supported using Webdriverio, however this is being actively developed.

@danielbate
Copy link
Contributor

After upgrading a few packages to better integrate with Vitest, still experiencing some issues.

  • Blocked headless mode
  • Polyfilling causes browser tests to run very slowly
  • Browser tests are considerably slower than the node runs, 20 seconds vs 5 minutes (not even the full test suite)
  • Intermittent issues with the birpc package that vitest utilises that occasionally causes timeouts
  • Still throwing Class extends value undefined is not a constructor or null for some package runs - searches dictate that it is regarding polyfill

Currently it seems evident that vitest in browser mode is still very much in development.

@arboleya
Copy link
Member

@danielbate Thank you for the updates.

I appreciate all the work you've put into this endeavor already.

Considering everything you already did, saw, and thought about — where should we go from here?

@arboleya arboleya added this to the Beetle milestone Dec 14, 2023
@arboleya arboleya modified the milestones: 2 - Beetle, 1 - Salamander Jan 8, 2024
@danielbate
Copy link
Contributor

danielbate commented Jan 10, 2024

Completion checklist:

There is currently an issue with polyfilling fs blocking browser testing fuel-gauge but this is WIP

@arboleya
Copy link
Member

Which tests from fuel-gauge do we need to run inside the browser?

@danielbate
Copy link
Contributor

danielbate commented Jan 10, 2024

I had assumed that we would want to test most of them on browser given that is our integration suite. However, if we only want a few specifically for browsers, there may be another (potentially quicker) approach I can take.

@arboleya
Copy link
Member

There are NodeJS-centric test utilities:

export const getFuelGaugeForcProject = (project: FuelGaugeProjectsEnum) =>
getForcProject<JsonAbi>({
projectDir: join(__dirname, 'forc-projects', project),
projectName: project,
});

export const getForcProject = <T = unknown>(params: IGetForcProjectParams) => {

👉 They are used everywhere, for example:

import { FuelGaugeProjectsEnum, getFuelGaugeForcProject } from '../test/fixtures';

@danielbate
Copy link
Contributor

Sorry yes I was referring more towards domains. But correct, right now most of fuel-gauge uses fs and I'm running into an issue with the polyfill, but I would like to get the majority of them working.

@arboleya
Copy link
Member

What do you mean by domains, and how would you make most of the tests work?

Maybe I'm missing the point or unable to clarify myself, but I can't see how fs polyfills could solve this as these test utilities are used to read files from the disk, like the abi.json and .bin.

The only way to ship these files to the browser is if we generate types for everything using Typegen. The JSON and hexlified bytecode would be included in the generated Typescript classes, making it possible to run them inside the browser.

This, however, would require a complete revamp of all tests, which concerns me.

@danielbate
Copy link
Contributor

Apologies, I am not explaining myself well.

Regarding domains I was referring to domains within our SDK, so whether we want to browser test crypto or address etc. specifically.

Regarding fs, this can be done with a build time polyfill, such as browserify-fs. This would then include the binaries and json in the bundle. However, I believe because it statically analyses the code, we can't use dynamic expressions that evaluate at run time, such as those used by getFuelGaugeForcProject. That is why I did have success when I was testing predicates that required specific bytecode.

I am still investigating the fs build time polyfill but may need changes to those functions, rather than all the tests.

@arboleya
Copy link
Member

Awesome! Thanks for clarifying. 🎯

@nedsalk
Copy link
Contributor

nedsalk commented Jan 15, 2024

@arboleya

The only way to ship these files to the browser is if we generate types for everything using Typegen. The JSON and hexlified bytecode would be included in the generated Typescript classes, making it possible to run them inside the browser.
This, however, would require a complete revamp of all tests, which concerns me.

This might be a worthwhile endeavor just because we'd be eating our own dog food.

@danielbate

Regarding fs, this can be done with a build time polyfill, such as browserify-fs. This would then include the binaries and json in the bundle. However, I believe because it statically analyses the code, we can't use dynamic expressions that evaluate at run time, such as those used by getFuelGaugeForcProject. That is why I did have success when I was testing predicates that required specific bytecode.

If this doesn't pan out as expected, another approach could be to spin up a server that'd serve the necessary files and have getFuelGaugeProject fetch the files from it if it's in a browser test environment. It'd make getFuelGaugeProject asynchronous, but that's not a big change to swallow.

@arboleya
Copy link
Member

Something else to have in mind:

@danielbate
Copy link
Contributor

danielbate commented Jan 18, 2024

As discussed above we currently have the following caveats.

For both environments, mocking functionality from the same file as the functionality under test will not work as expected. This is a caveat to mocking with Vitest.

Additionally, currently module mocking is not supported in a browser environment. Mocked functions (vi.fn) and spies (vi.spy) work as expected. The issue remains open from Vitest and there was discussion to support it with the playwright devs.

@arboleya
Copy link
Member

@danielbate Any clue as to how it works locally?

@danielbate
Copy link
Contributor

@arboleya I'm afraid it does not work locally for myself or @nedsalk, and the issue is also replicated in CI. Please could you try a fresh install / build and report back?

@arboleya
Copy link
Member

My bad. I initially ran the wrong test file.

Now I'm getting the same error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants