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

Add tests #26

Open
fleon opened this issue Jul 17, 2020 · 5 comments
Open

Add tests #26

fleon opened this issue Jul 17, 2020 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@fleon
Copy link
Collaborator

fleon commented Jul 17, 2020

I realized that all the tests in the test directory are actually tests for the yaml-language-server and there are actually no tests for this repo. There should defintely be test coverage for the yaml plugin we're creating here.

@remcohaszing
Copy link
Owner

I would like to see tests too, although I’m not sure if unittests are the right way to test this. I think this would require a lot of stubbing which doesn’t really test the functionality in the end.

Perhaps end to end tests using Cypress are a good option?

@remcohaszing
Copy link
Owner

remcohaszing commented May 1, 2022

Thanks @domsew for adding an initial test setup using Playwright!

I’m not closing this issue yet, as we should add tests for every feature provided. By this I mean we need to add tests for every language feature provided (make sure hovers info, links, etc work). We don’t need to add tests for upstream yaml-language-server features (e.g. for every JSON schema type).

@domsew
Copy link

domsew commented Jun 18, 2022

Hello,

I found some time to follow up the code coverage topic from this question. Basically there are two possible approaches:

  • usage V8 code coverage,
  • creation dedicated build definition with pre instrumented code for Istanbul.js learn more.

I tested both approaches and they produce slightly different results. You can track my experiments here. And here you can find four example reports I got: coverage.zip.

In case of V8 the setup is pretty simple. I extended the example from the docs by adding code to generate and save report (function saveV8Coverage, results in /v8 folder).

I also made an attempt to instrument code for Istanbul.js with following methods:

  • webpack with babel-loader and babel-plugin-istanbul, launched in two ways:
    • npx webpack -c .\test-experiments\webpack.config.js (/webpack),
    • npm --workspace test-experiments run build (/webpack2 - useless),
  • I created custom istanbulPlugin for esbuild (/esbuild - results are shifted),
  • instrument esbuild's output bundle with nyc CLI npx nyc instrument ./test/dist/ ./test/dist2/ (it didn't work at all).

Unfortunately in both scenarios I was unable to get code coverage of Web Workers.

Moreover, I tested Cypress. In comparison to Playwright, it has dedicated code coverage plugin, but doesn't support V8 coverage and custom instrumentation is still required. Futhermore Cypress requires more boilerplate code and doesn't support native events like hover - additional dependencies are required. Conclusion: Playwright wins.

Before I prepare a PR I need to ask the following questions:

  • What do you think about my work? Have you any suggestions or recommendations?
  • Is the code coverage report obtained from V8 is enough for us? Or maybe should we put more effort to investigate Istanbul.js.
  • Should I put test dependencies into separate npm workspace?
  • In what way the generated coverage report will be used next? Do we want coverage badge coveralls.io (text-lcov reporter) or something more advanced?

@domsew
Copy link

domsew commented Jun 23, 2022

@remcohaszing could you take a look at my comment? I also want to mention that there is an open issue about
playwright coverage

@remcohaszing
Copy link
Owner

Sorry, I missed those last comments somehow.

I created playwright-monaco, a project dedicated to testing Monaco editor integrations.

Also monaco-yaml now uses monaco-languageserver-types and monaco-worker-manager. This saves a lot of boilerplate code, so less coverage to verify. Although I would love to have code coverage, I worry less about it now.

I welcome anyone to contribute more tests. It would be nice to have at least one test per provider. That would cover most code already. For inspiration you may want to have a look at the @mdx-js/monaco tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants