search

decentralized-identity / veramo

A JavaScript Framework for Verifiable Data
https://veramo.io
Apache License 2.0
432 stars 131 forks source link

[proposal] Create a test harness to test @veramo/core in a web frontent #799

Closed mirceanis closed 2 years ago

mirceanis commented 2 years ago

Veramo is missing an automated way of checking which packages work in a web environment.

We need a sample repository that has a frontend app (React ?) that uses @veramo/core and a few veramo plugins and runs through some methods of the plugins as an integration test. Ideally, the frontend app should mimic a setup from one of the guides.

The solution should contain github workflows that download the latest relevant @veramo/* packages, build the app and then run it in a headless browser to check that everything works. The solution can be either a separate repository or a PR to this repository.

This relates to #372

mirceanis commented 2 years ago

perhaps a guide like this would help: https://jkrsp.com/automated-browser-tests-with-github-actions/

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 500.0 DAI (500.0 USD @ $1.0/DAI) attached to it as part of the https://github.com/veramolabs fund.

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 3 weeks, 6 days from now. Please review their action plans below:

1) stackedq has been approved to start work.

I think I can get this done. Estimated time of delivery: 3 days

Learn more on the Gitcoin Issue Details page.

stackedq commented 2 years ago

Hey @mirceanis, I'm working on this issue. You can watch the progress here: https://github.com/stackedq/veramo-react-test I've already created github workflow which appears in "actions" tab of the repo. Currently "@veramo/core" and "@veramo/did-resolver" are included in the tests.

I want to know which plugins you want to be included into this repo, please either tell me here or create an issue there so I can continue to develop. Thanks

mirceanis commented 2 years ago

Hi @stackedq, eventually we will need to go through all the plugins, but at the moment not all of them are suitable for web, because of database limitations.

Until we find a suitable implementation for working with TypeORM directly in a frontend, or for replacing the @veramo/data-store#IDataStoreORM with something else, we will want to test everything that doesn't depend on that.

I suppose that @veramo/did-manager using MemoryDidStore and @veramo/key-manager using MemoryKeyStore and @veramo/credential-w3c would be nice to have.

At first, though, it's more important to be able to automate these tests and integrate them into our existing CI/CD process, so I guess you don't need to worry about other plugins for now. For this CI/CD integration, I suppose that the react-app you are building would have to live in this repository and use the veramo packages by relative path instead of NPM.

stackedq commented 2 years ago

@mirceanis Integration part is done, I forked the repo here: https://github.com/stackedq/veramo at the branch: tests/browser Now moving to the plugins.

mirceanis commented 2 years ago

@stackedq That's great! I saw that you opened and closed a PR with this. So is your plan now to add some more plugins to the tests and then re-open that PR to add this to our testing codebase?

stackedq commented 2 years ago

I'm actually working on a workaround to use predefined shared tests of tests directory to be used in browser tests, this way you don't need to worry about writing duplicate test cases.

mirceanis commented 2 years ago

That's awesome! I'd love to know what the blockers are, so feel free to post here if something appears.

I suspect you might have some trouble with TypeORM / anything sql in the frontend, but if you find a neat solution to that, it would be gold!

stackedq commented 2 years ago

@mirceanis So I integrated with existing test cases, here's a problem though, jest by default exits with code 1 if any test were failed, since some parts of the plugins are not working in the browser env, jest is exiting with code 1 (error). then GitHub workflows will stop the process, take a look at here: https://github.com/stackedq/veramo/runs/5063626896?check_suite_focus=true.

stackedq commented 2 years ago

@mirceanis I added continue-on-error: true to Github ci workflow, this will run tests in a headless browser, but doesn't care about all of them to be passed. this way you can see failing tests while the repo stays in a healthy state.

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 500.0 DAI (500.0 USD @ $1.0/DAI) has been submitted by:

  1. @stackedq

@mirceanis please take a look at the submitted work:

mirceanis commented 2 years ago

Nice work so far!

@mirceanis So I integrated with existing test cases, here's a problem though, jest by default exits with code 1 if any test were failed, since some parts of the plugins are not working in the browser env, jest is exiting with code 1 (error). then GitHub workflows will stop the process, take a look at here: https://github.com/stackedq/veramo/runs/5063626896?check_suite_focus=true.

Yeah, those tests expect that the agent being used has all the bells and whistles. I see that in your agent setup you removed some DID providers and some plugins compared to the other setups.

We need failing tests to halt the build when something breaks because otherwise we cannot rely on automation, so we can't use continue-on-error: true anywhere.

Instead of running all the tests and manually inspecting the failing ones, a better approach for us would be to start with a subset of tests that do their job and then grow the set of tests to cover more and more code. It seems that key-manager tests seem to all pass using your setup, so let's restrict to those tests.

I also saw that you removed the test that ran through the sample react-app. Can you bring that back?

It is also surprising to see that you aren't able to use did:key. Can you please elaborate on the "Field 'browser' doesn't contain a valid alias configuration" error?

You're definitely getting the bounty, since you put in this work, but I'd like it to be in a clean state before merging.

stackedq commented 2 years ago

It is also surprising to see that you aren't able to use did:key. Can you please elaborate on the "Field 'browser' doesn't contain a valid alias configuration" error?

More info on that:

Module not found: Error: Can't resolve 'stream' in '/react-sample/node_modules/cipher-base'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
    - install 'stream-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "stream": false }

I also saw that you removed the test that ran through the sample react-app. Can you bring that back?

I didn't remove it, two test cases run on the create-react-app to make sure the agent is correctly created in browser-env, however for the rest of the tests, the agent is being created in the jest-puppeteer-jsdom environment which will be the same as the agent created in cra.

PR updated with the requested changes.

mirceanis commented 2 years ago

fixed in #809

bounty paid, thank you for your contribution!

stackedq commented 2 years ago

Please contact me if something went sideways

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 500.0 DAI (500.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @stackedq.

mirceanis commented 2 years ago

Please contact me if something went sideways

We can keep in touch on our discord We'll be posting some more bounties in the near future.