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

docs(testing): re-organize testing up to allow adding more frameworks #1344

Merged
merged 17 commits into from
Feb 13, 2024

Conversation

christian-bromann
Copy link
Member

Based on an internal design doc we'ld like to re-organise the Stencil docs around testing so we can add more frameworks moving on.

@christian-bromann christian-bromann requested a review from a team as a code owner February 5, 2024 19:31
@christian-bromann christian-bromann requested review from rwaskiewicz and alicewriteswrongs and removed request for a team February 5, 2024 19:31
Copy link

vercel bot commented Feb 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stencil-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 13, 2024 5:27pm

Copy link
Contributor

@rwaskiewicz rwaskiewicz left a comment

Choose a reason for hiding this comment

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

On additional ask - Can you please also update the acceptance criteria for this ticket?

Some additional Stencil specific options may be set here as well for configuring the e2e tests:

```tsx reference title=""
https://github.com/ionic-team/stencil/blob/278c336166e27d329fffb686f2ecb3185d156696/src/declarations/stencil-public-compiler.ts#L1817-L1963
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite expect us to have an answer to this right now, but want to ask so the team as a whole is thinking about this - how do we maintain this long term? Specifically, if I'm a developer that:

  • updates any code highlighted here (so the content has changed and the SHA + line numbers have to change)
  • moves this code to another file (so the sha, file path, and perhaps line numbers have to change)

How do I know that this needs to be updated?

I think this is definitely better than what we have today (a static list that I couldn't tell you when it was last updated without looking at a git blame), but do want to start thinking through the next steps here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It will be hard to put an automatic mechanism in place for updating sha's without manually reviewing the code examples. At least this way it becomes an intentional process. I can see how having thousands of such examples will become out of date quickly and updating them manually will become tedious. However IMHO there is generally no need to update the examples until there is a reason to do so.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed - I've spent a bit of time thinking about this myself (before I knew of this project) 😆 😉

I can see a world where we implement some kind of programmatic check against the current SHA and the primary branch (main) and see if the content is different, but I think that's outside the scope of this PR. Just want to see if/what others thought and to plant that idea seed.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is amazing!

I agree that pointing to a SHA and pulling lines out of a file is much better than what we do now, and I'd be fine with doing this and maybe putting a ticket into the backlog to consider whether we want to add more tooling around this in the future.

If we had a lot of these in the site we could have something which did something like

  1. crawl through the .md files to find all such references
  2. figure out which version the given .md is for (i.e. if in docs it's HEAD, if it's versioned_docs/version-v4.11 then it's 4.11.x, etc)
  3. grab the git tag in the Stencil repo for that version and do some git wizardry to determine whether the referenced SHA is still the latest commit which changed the referenced lines up to the point of the version that the doc page is specific to
  4. if not, fail a CI check on the PR, so that we know we have to update the lines (I think this could be a manual thing, since the starting line and offset could easily change)

That wouldn't proactively push updates to the stencil site when there's a change in the Stencil repo, but I think that 1) eventual consistency is totally fine here and 2) the stencil site does get rebuilt frequently because of edits and dependency updates and whatnot so I don't think it could ever get too far out of date.

However! It's probably a YAGNI / overengineering thing to even really think about tooling like this, especially when this solution + no tooling at all is still a big upgrade over our current copy-paste based solution

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds like a good Jedi day project 😉

docs/testing/stencil-testrunner/02-config.md Outdated Show resolved Hide resolved
docs/testing/stencil-testrunner/_category_.json Outdated Show resolved Hide resolved
"resolved": "https://registry.npmjs.org/docusaurus-theme-github-codeblock/-/docusaurus-theme-github-codeblock-2.0.2.tgz",
"integrity": "sha512-H2WoQPWOLjGZO6KS58Gsd+eUVjTFJemkReiSSu9chqokyLc/3Ih3+zPRYfuEZ/HsDvSMIarf7CNcp+Vt+/G+ig==",
"dependencies": {
"@docusaurus/types": "^3.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I was wondering what this would depend on - love the 1 prod dep

@@ -23,6 +23,7 @@
"@docusaurus/remark-plugin-npm2yarn": "^3.1.0",
"@ionic-docs/preset-classic": "^1.1.0",
"clsx": "^2.0.0",
"docusaurus-theme-github-codeblock": "^2.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if we build the site without an internet connection? e.g. What's the failure mode look like if I'm doing a local build and don't have wifi access?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now it fails 😞 raised an issue christian-bromann/docusaurus-theme-github-codeblock#141 and can look into a fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way it could store both the information needed to re-fetch the lines from github and a fallback right in the markdown? That would make it possible to build offline without too much trouble I think. I don't know too much about how docusaurus plugins are structured to know if there would be flexibility to do something like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't been able to look into this yet but I hope Docusaurus allows to pull the code as part of the server side generation process.

docs/testing/config.md Show resolved Hide resolved
@christian-bromann
Copy link
Member Author

Should we apply this re-org only to the latest version or to all v4.x and v3.x?

@rwaskiewicz
Copy link
Contributor

@christian-bromann I was thinking about that myself. I did a diff of the testing subdir in all v4 directories and v3. There are very minute differences (two total) that I think we could simply cp the contents of docs/testing to each versioned directory and be fine.

Longer term, I'm considering what it'll look like to "squash" all these minor versioned docs into a rolling v4 release. IDK if that'll be my JEDI day project or not next week, but the level of friction to otherwise backport these things is getting higher than I'd like

@christian-bromann
Copy link
Member Author

Added commit to deprecated screenshot diffing capabilities:

Screenshot 2024-02-06 at 4 31 06 PM

@christian-bromann
Copy link
Member Author

Raised #1348 with a massive diff which contains all changes made here to all other versions.

@rwaskiewicz
Copy link
Contributor

I'd personally vote we hold off on deprecating the screenshot functionality for right now. I'd prefer we have our docs for WebDriverIO & Playwright available, as well as an initial pass at the Playwright Adapter available so that we can provide folks with a little more guidance. I don't want this to feel like a "rug pull" for folks that feel they "need" to go find a replacement sooner than later.

@rwaskiewicz
Copy link
Contributor

Adding the entire team to this review - I think this is a big enough change that everyone should have a look at what's being proposed here

@christian-bromann
Copy link
Member Author

I'd personally vote we hold off on deprecating the screenshot functionality for right now.

Makes sense. Reverted the change. I am currently working on adding visual testing docs for the new WebdriverIO docs so we have alternatives in the pipeline.

Copy link
Contributor

@alicewriteswrongs alicewriteswrongs left a comment

Choose a reason for hiding this comment

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

a few questions and comments

docs/testing/01-overview.md Outdated Show resolved Hide resolved
Comment on lines +8 to +12
Stencil has a built-in test runner that uses [Jest](https://jestjs.io/) and [Puppeteer](https://pptr.dev/) as its testing libraries, and allows developers to install both libraries using their preferred package manager.

If you created a project using `npm init stencil`, these libraries were installed for you. Depending on when your project was created, you may or may not have the latest supported version installed.

To view current version support for both Jest and Puppeteer, please see the [Stencil support policy for testing libraries](../../reference/support-policy.md#testing-libraries).
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if we might want to add an info admonition in here calling out that you need to install jest and puppeteer if you don't have them - I imagine almost all new Stencil projects are going to be started with npm init stencil so users won't run into this, but still could be worth a call-out I think

docs/testing/config.md Show resolved Hide resolved
Some additional Stencil specific options may be set here as well for configuring the e2e tests:

```tsx reference title=""
https://github.com/ionic-team/stencil/blob/278c336166e27d329fffb686f2ecb3185d156696/src/declarations/stencil-public-compiler.ts#L1817-L1963
Copy link
Contributor

Choose a reason for hiding this comment

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

this is amazing!

I agree that pointing to a SHA and pulling lines out of a file is much better than what we do now, and I'd be fine with doing this and maybe putting a ticket into the backlog to consider whether we want to add more tooling around this in the future.

If we had a lot of these in the site we could have something which did something like

  1. crawl through the .md files to find all such references
  2. figure out which version the given .md is for (i.e. if in docs it's HEAD, if it's versioned_docs/version-v4.11 then it's 4.11.x, etc)
  3. grab the git tag in the Stencil repo for that version and do some git wizardry to determine whether the referenced SHA is still the latest commit which changed the referenced lines up to the point of the version that the doc page is specific to
  4. if not, fail a CI check on the PR, so that we know we have to update the lines (I think this could be a manual thing, since the starting line and offset could easily change)

That wouldn't proactively push updates to the stencil site when there's a change in the Stencil repo, but I think that 1) eventual consistency is totally fine here and 2) the stencil site does get rebuilt frequently because of edits and dependency updates and whatnot so I don't think it could ever get too far out of date.

However! It's probably a YAGNI / overengineering thing to even really think about tooling like this, especially when this solution + no tooling at all is still a big upgrade over our current copy-paste based solution

@@ -23,6 +23,7 @@
"@docusaurus/remark-plugin-npm2yarn": "^3.1.0",
"@ionic-docs/preset-classic": "^1.1.0",
"clsx": "^2.0.0",
"docusaurus-theme-github-codeblock": "^2.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way it could store both the information needed to re-fetch the lines from github and a fallback right in the markdown? That would make it possible to build offline without too much trouble I think. I don't know too much about how docusaurus plugins are structured to know if there would be flexibility to do something like this.

Copy link
Member

@tanner-reits tanner-reits left a comment

Choose a reason for hiding this comment

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

Couple small grammatical fixes, but nothing blocking. Nice job! That Github code block viewer is snazzy 👏

docs/testing/01-overview.md Outdated Show resolved Hide resolved

If you created a project using `npm init stencil`, these libraries were installed for you. Depending on when your project was created, you may or may not have the latest supported version installed.

To view current version support for both Jest and Puppeteer, please see the [Stencil support policy for testing libraries](../../reference/support-policy.md#testing-libraries).
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice to pull the testing support policy closer to these docs (since we'll be adding most matrices as new adapters come into play), but not needed now

docs/testing/stencil-testrunner/01-overview.md Outdated Show resolved Hide resolved
docs/testing/stencil-testrunner/03-unit-testing.md Outdated Show resolved Hide resolved
* docs(testing): adding WebdriverIO docs

* add webdriverio to cspell list

* add visual docs

* minor wording

* prettier update

* Update docs/testing/webdriverio/01-overview.md

Co-authored-by: Alice Pote <[email protected]>

* fix example

* Update docs/testing/webdriverio/03-mocking.md

Co-authored-by: Tanner Reits <[email protected]>

* Update docs/testing/webdriverio/01-overview.md

Co-authored-by: Tanner Reits <[email protected]>

* use npm2yarn

* fix admonition

* pr feedback

* Update docs/testing/webdriverio/01-overview.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* Update docs/testing/webdriverio/01-overview.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* Update docs/testing/webdriverio/04-visual-testing.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Ryan Waskiewicz <[email protected]>

* add description on what a service is

* added a section on integration with Stencil testing capabilities

* minor tweak to code example

* add section on components list

* add admonition

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Alice Pote <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Alice Pote <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Alice Pote <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Alice Pote <[email protected]>

* Update docs/testing/webdriverio/02-unit-testing.md

Co-authored-by: Alice Pote <[email protected]>

* fix code example meeting

* add note how to run both test runners

---------

Co-authored-by: Alice Pote <[email protected]>
Co-authored-by: Tanner Reits <[email protected]>
Co-authored-by: Ryan Waskiewicz <[email protected]>
@christian-bromann christian-bromann merged commit a1a48a3 into main Feb 13, 2024
5 checks passed
@christian-bromann christian-bromann deleted the cb/testing-docs-reorg branch February 13, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants