-
Notifications
You must be signed in to change notification settings - Fork 41
test(hooks): add unit tests for @asgardeo/react hooks to cover edge c… #220
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
base: main
Are you sure you want to change the base?
Conversation
|
✅ Tests run locally with 27 passed and 2 existing failures (not from new tests):
|
|
Hey @Kavidu23, Thanks for the PR. Couple of things:
|
|
I can still see that there's a failure in the CI. Check here: https://github.com/asgardeo/javascript/actions/runs/18818343585/job/53722190236?pr=220 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need these files?
- Replaced Playwright browser testing config with Vitest jsdom environment - Removed unnecessary screenshots and Playwright setup - Simplified test configuration to match other packages and ensure CI compatibility
- replaced Playwright browser config with Vitest jsdom - removed unused screenshots and Playwright dependencies - aligned React package test setup with other modules for CI stability
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. |
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| browser: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
║ Looks like Playwright Test or Playwright was just installed or updated. ║
║ Please run the following command to download new browsers: ║
║ ║
║ pnpm exec playwright install ║
║ ║
║ <3 Playwright Team
IMO, the previous issue was due to not having the playright dependency. Can't you try adding that and keep playright as the provider. Coz jsdom will go obsolete in the future and we need to stick with playwrite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the indentation of these files.
We use 2 spaces.
I hope you have read the CONTRIBUTING guideline and installed the necessary plugins: https://github.com/brionmario/javascript/blob/next/CONTRIBUTING.md#development-tools
|
Thanks @brionmario 🙏 I ran all the tests locally for the packages/react directory after executing pnpm exec playwright install, and everything passed successfully. The error message in the CI log seems to occur because the Playwright browsers aren’t installed in the CI environment — locally, the Playwright command downloaded the required Chromium binaries, and tests ran without any issue. The screenshots you saw were auto-generated by Playwright during the test execution. I’m still quite new to Playwright and browser-based testing setups, so I double-checked the configuration using my code editor’s AI agent. It confirmed that the root cause is missing Playwright browser binaries in the CI runner and explained two approaches: Option A: Keep Playwright and add pnpm exec playwright install in the CI pipeline before running tests. Option B: Use jsdom for unit tests . Since some other packages here already use jsdom, I initially decided to switch to it. |

Purpose
This PR adds unit tests for @asgardeo/react hooks to cover edge cases and improve test coverage.
Hooks tested: useBranding, useBrowserUrl, useForm, useTranslation.
The tests cover:
Normal input, boundary, and invalid cases
Error handling and default behaviors
Context absence scenarios
Screenshots generated by Vitest are included but some may appear blank (no visual output for non-UI hooks).
Related Issues
#167 — Add unit test coverage for untested code paths in @asgardeo/react SDK
Related PRs
N/A
Checklist
Followed the CONTRIBUTING
guidelines
Unit tests provided for hooks
Security checks
Followed secure coding standards in WSO2 Secure Engineering Guidelines
Confirmed that this PR doesn’t commit any keys, passwords, tokens, usernames, or other secrets