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

web: unit tests for the simple things, with fixes that the tests revealed #11633

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

kensternberg-authentik
Copy link
Contributor

web: test the simple things. Fix what the tests revealed.

What

  • Move EmptyState.test.ts into the ./tests folder.
  • Provide unit tests for:
    • Alert
    • Divider
    • Expand
    • Label
    • LoadingOverlay
  • Give all tested items an Interface and a functional variant for rendering
  • Give Label an alternative syntax for declaring alert levels
  • Remove the slot name in LoadingOverlay
    • Change the slot call in ./enterprise/rac/index.ts to not need the slot name as well
  • Change the attribute names topMost, textOpen, and textClosed to topmost, text-open, and text-closed, respectively.
    • Change locations in the code where those are used to correspond

Why

Why interfaces:

Provides another check on the input/output boundaries of our elements, gives Storybook and WebdriverIO another validation to check, and guarantees any rendering functions cannot be passed invalid property names.

Why functions for rendering:

Providing functions for rendering gets us one step closer to dynamically defining our forms-in-code at runtime without losing any type safety.

Why rename the attributes:

A very subtle bug: Element:setAttribute() automatically “converts an attribute name to all lower-case when called on an HTML element in an HTML document.” The three attributes renamed are all treated as attributes, either classic boolean or stringly-typed attributes, and attempting to manipulate them with setAttribute() will fail.

All of these attributes are presentational; none of them end up in a transaction with the back-end, so kebab-to-camel conversions are not a concern.

Also, “topmost” is one word.

Why remove the slot name:

Because there was only one slot. A name is not needed.

Checklist

  • The code has been formatted (make web)
  • The web end-to-end tests are passing:
  • Spec Files: 5 passed, 5 total (100% completed) in 00:01:19
  • The web unit tests are passing:
  • Spec Files: 14 passed, 14 total (100% completed) in 00:00:47
    • ☝ Up from just just nine a week ago.

- Move `EmptyState.test.ts` into the `./tests` folder.
- Provide unit tests for:
  - Alert
  - Divider
  - Expand
  - Label
  - LoadingOverlay
- Give all tested items an Interface and a functional variant for rendering
- Give Label an alternative syntax for declaring alert levels
- Remove the slot name in LoadingOverlay
  - Change the slot call in `./enterprise/rac/index.ts` to not need the slot name as well
- Change the attribute names `topMost`, `textOpen`, and `textClosed` to `topmost`, `text-open`, and
  `text-closed`, respectively.
  - Change locations in the code where those are used to correspond

** Why interfaces: **

Provides another check on the input/output boundaries of our elements, gives Storybook and
WebdriverIO another validation to check, and guarantees any rendering functions cannot be passed
invalid property names.

** Why functions for rendering: **

Providing functions for rendering gets us one step closer to dynamically defining our forms-in-code
at runtime without losing any type safety.

** Why rename the attributes: **

A *very* subtle bug:
[Element:setAttribute()](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute)
automatically "converts an attribute name to all lower-case when called on an HTML element in an
HTML document." The three attributes renamed are all treated *as* attributes, either classic boolean
or stringly-typed attributes, and attempting to manipulate them with `setAttribute()` will fail.

All of these attributes are presentational; none of them end up in a transaction with the back-end,
so kebab-to-camel conversions are not a concern.

Also, ["topmost" is one word](https://www.merriam-webster.com/dictionary/topmost).

** Why remove the slot name: **

Because there was only one slot.  A name is not needed.
Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for authentik-docs ready!

Name Link
🔨 Latest commit 701ac4d
🔍 Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/6705a08437297f00083da84b
😎 Deploy Preview https://deploy-preview-11633--authentik-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for authentik-storybook ready!

Name Link
🔨 Latest commit 701ac4d
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/6705a0849722f2000855ac43
😎 Deploy Preview https://deploy-preview-11633--authentik-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kensternberg-authentik kensternberg-authentik marked this pull request as ready for review October 8, 2024 21:16
@kensternberg-authentik kensternberg-authentik requested a review from a team as a code owner October 8, 2024 21:16
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.76%. Comparing base (9200a59) to head (701ac4d).
Report is 15 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11633   +/-   ##
=======================================
  Coverage   92.75%   92.76%           
=======================================
  Files         736      736           
  Lines       36542    36542           
=======================================
+ Hits        33896    33898    +2     
+ Misses       2646     2644    -2     
Flag Coverage Δ
e2e 49.26% <ø> (-0.01%) ⬇️
integration 24.98% <ø> (ø)
unit 90.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Oct 8, 2024

authentik PR Installation instructions

Instructions for docker-compose

Add the following block to your .env file:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-701ac4dc94fd8b5d37e3a1ac4e378cda228691f7
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

For arm64, use these values:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-701ac4dc94fd8b5d37e3a1ac4e378cda228691f7-arm64
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

Afterwards, run the upgrade commands from the latest release notes.

Instructions for Kubernetes

Add the following block to your values.yml file:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
global:
    image:
        repository: ghcr.io/goauthentik/dev-server
        tag: gh-701ac4dc94fd8b5d37e3a1ac4e378cda228691f7

For arm64, use these values:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
global:
    image:
        repository: ghcr.io/goauthentik/dev-server
        tag: gh-701ac4dc94fd8b5d37e3a1ac4e378cda228691f7-arm64

Afterwards, run the upgrade commands from the latest release notes.

@@ -68,7 +68,7 @@ export class LibraryApplication extends AKElement {
renderExpansion(application: Application) {
const me = rootInterface<UserInterface>()?.me;

return html`<ak-expand textOpen=${msg("Less details")} textClosed=${msg("More details")}>
return html`<ak-expand text-open=${msg("Less details")} text-closed=${msg("More details")}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works as expected:

image

kensternberg-authentik added a commit that referenced this pull request Oct 10, 2024
- Move the Element's stories into a `./stories` folder
- Provide stories for (these are the same ones "provided tests for" in the [previous
  PR](#11633))
  - Alert
  - Divider
  - Expand
  - Label
  - LoadingOverlay
- Provide Storybook documentation for:
  - AppIcon
  - ActionButton
  - AggregateCard
  - AggregatePromiseCard
  - QuickActionsCard
  - Alert
  - Divider
  - EmptyState
  - Expand
  - Label
  - LoadingOverlay
  - ApplicationEmptyState
- Fix a bug in LoadingOverlay; naming error in nested slots caused any message attached to the
  overlay to not sow up correctly.
- Revise AppIcon to be independent of authentik; it just cares if the data has a name or an icon
  reference, it does not need to know about `Application` objects. As such, it's an *element*, not a
  *component*, and I've moved it into the right location, and updated the few places it is used to
  match.
@kensternberg-authentik kensternberg-authentik merged commit 058a388 into main Oct 10, 2024
65 checks passed
@kensternberg-authentik kensternberg-authentik deleted the web/testing/element-unit-tests-1 branch October 10, 2024 22:14
kensternberg-authentik added a commit that referenced this pull request Oct 10, 2024
* main:
  web: unit tests for the simple things, with fixes that the tests revealed (#11633)
kensternberg-authentik added a commit that referenced this pull request Oct 14, 2024
* Added tests and refinements as tests indicate.

* Building out the test suite.

* web: test the simple things. Fix what the tests revealed.

- Move `EmptyState.test.ts` into the `./tests` folder.
- Provide unit tests for:
  - Alert
  - Divider
  - Expand
  - Label
  - LoadingOverlay
- Give all tested items an Interface and a functional variant for rendering
- Give Label an alternative syntax for declaring alert levels
- Remove the slot name in LoadingOverlay
  - Change the slot call in `./enterprise/rac/index.ts` to not need the slot name as well
- Change the attribute names `topMost`, `textOpen`, and `textClosed` to `topmost`, `text-open`, and
  `text-closed`, respectively.
  - Change locations in the code where those are used to correspond

** Why interfaces: **

Provides another check on the input/output boundaries of our elements, gives Storybook and
WebdriverIO another validation to check, and guarantees any rendering functions cannot be passed
invalid property names.

** Why functions for rendering: **

Providing functions for rendering gets us one step closer to dynamically defining our forms-in-code
at runtime without losing any type safety.

** Why rename the attributes: **

A *very* subtle bug:
[Element:setAttribute()](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute)
automatically "converts an attribute name to all lower-case when called on an HTML element in an
HTML document." The three attributes renamed are all treated *as* attributes, either classic boolean
or stringly-typed attributes, and attempting to manipulate them with `setAttribute()` will fail.

All of these attributes are presentational; none of them end up in a transaction with the back-end,
so kebab-to-camel conversions are not a concern.

Also, ["topmost" is one word](https://www.merriam-webster.com/dictionary/topmost).

** Why remove the slot name: **

Because there was only one slot.  A name is not needed.

* Fix minor spelling error.

* First pass at a custom, styled input object.

* .

* web: Demo the simple things. Fix things the Demo says need fixing.

- Move the Element's stories into a `./stories` folder
- Provide stories for (these are the same ones "provided tests for" in the [previous
  PR](#11633))
  - Alert
  - Divider
  - Expand
  - Label
  - LoadingOverlay
- Provide Storybook documentation for:
  - AppIcon
  - ActionButton
  - AggregateCard
  - AggregatePromiseCard
  - QuickActionsCard
  - Alert
  - Divider
  - EmptyState
  - Expand
  - Label
  - LoadingOverlay
  - ApplicationEmptyState
- Fix a bug in LoadingOverlay; naming error in nested slots caused any message attached to the
  overlay to not sow up correctly.
- Revise AppIcon to be independent of authentik; it just cares if the data has a name or an icon
  reference, it does not need to know about `Application` objects. As such, it's an *element*, not a
  *component*, and I've moved it into the right location, and updated the few places it is used to
  match.

* Prettier has opinions with which I sometimes diverge.

* Found a bug! Although pf-m-xl was defined as a legal size, there was no code to handle drawing something XL!

* Found a few typos and incorrect API descriptions.
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.

2 participants