feat: adds tests, sonar config and workflows #6
Conversation
WalkthroughAdds GitHub Actions workflows: build, test, SonarQube, and release (Node.js 22.x, pnpm, frozen lockfile; release publishes packages/js to npm using a secret token and --provenance). Adds sonar-project.properties and VSCode SonarLint connected-mode settings. Introduces test and coverage scripts at root and in packages/js, adds vitest and @vitest/coverage-v8, updates vite, and adds Vitest test configuration to vite.config.ts. Adds comprehensive unit tests for the JS proxy and loader (packages/js/src/*.test.ts) and new TurboRepo tasks: test, test:coverage, @formbricks/js:test, and playground:build. Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-09-24T16:54:10.943ZApplied to files:
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.github/workflows/build.yml(1 hunks).github/workflows/release.yml(1 hunks).github/workflows/sonarqube.yml(1 hunks).github/workflows/test.yml(1 hunks).vscode/settings.json(1 hunks)package.json(1 hunks)packages/js/package.json(1 hunks)packages/js/src/__tests__/index.test.ts(1 hunks)packages/js/src/__tests__/load-formbricks.test.ts(1 hunks)packages/js/src/__tests__/setup.ts(1 hunks)packages/js/vite.config.ts(2 hunks)sonar-project.properties(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/js/src/__tests__/load-formbricks.test.ts (1)
packages/js/src/lib/load-formbricks.ts (1)
loadFormbricksToProxy(119-130)
🔇 Additional comments (5)
packages/js/vite.config.ts (2)
2-2: Correct import for Vitest config.Switching to
vitest/configis appropriate to enable thetestblock.
19-24: DTS plugin usage looks good.
rollupTypes: trueandbundledPackagesare set correctly for publishing types..vscode/settings.json (1)
7-10: SonarLint connected mode config LGTM.Connection and project key entries are correctly placed at root.
package.json (1)
9-11: Approve Turbo test scripts configuration
Turbo tasks"test"and"test:coverage"are defined in turbo.json (lines 29, 32) and implemented in packages/js/package.json (lines 40–41).packages/js/src/__tests__/load-formbricks.test.ts (1)
115-137: Nice coverage of script injection and setup pathValidates script attributes and that setup is called; good end-to-end behavior check.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(1 hunks).github/workflows/release.yml(1 hunks).github/workflows/sonarqube.yml(1 hunks).github/workflows/test.yml(1 hunks)
🔇 Additional comments (10)
.github/workflows/release.yml (4)
18-23: Enable pnpm cache and always-auth on setup-nodeSpeeds installs and ensures npm auth on all requests.
- name: Setup Node.js 22.x uses: actions/setup-node@v4 with: node-version: 22.x registry-url: "https://registry.npmjs.org" + cache: "pnpm" + always-auth: true
24-26: Pin pnpm runtime version for reproducibilityPrevents CI breakage on new pnpm releases.
- name: Install pnpm - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + with: + version: 9
30-34: Use working-directory instead of cd for buildMore idiomatic and avoids path/subshell pitfalls.
- - name: Build package - run: | - cd packages/js - pnpm build + - name: Build package + working-directory: packages/js + run: pnpm build
38-43: Add version guard and use working-directory for publishPrevents publishing when tag and package.json version diverge; simplifies path handling.
- - name: Publish to npm - run: | - cd packages/js - npm publish --access public --provenance - env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + - name: Verify tag matches package version + id: version_check + working-directory: packages/js + run: | + PKG_VERSION=$(node -p "require('./package.json').version") + TAG="${GITHUB_REF#refs/tags/}" + if [ "$PKG_VERSION" != "$TAG" ] && [ "v$PKG_VERSION" != "$TAG" ]; then + echo "Release tag ($TAG) does not match packages/js version ($PKG_VERSION)" + exit 1 + fi + + - name: Publish to npm + if: ${{ success() }} + working-directory: packages/js + run: npm publish --access public --provenance + env: + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}.github/workflows/build.yml (2)
14-25: Add Node version matrix and pnpm cacheBuild against supported Node versions and speed up CI.
jobs: build: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + node: [18.x, 20.x, 22.x] @@ - - name: Setup Node.js 22.x + - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 22.x + node-version: ${{ matrix.node }} + cache: 'pnpm'
26-27: Pin pnpm runtime versionAvoids surprises from pnpm upgrades.
- name: Install pnpm - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + with: + version: 9.github/workflows/test.yml (2)
14-25: Test on Node matrix and enable pnpm cacheCovers consumer environments and speeds installs.
jobs: test: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + node: [18.x, 20.x, 22.x] @@ - - name: Setup Node.js 22.x + - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 22.x + node-version: ${{ matrix.node }} + cache: 'pnpm'
25-26: Pin pnpm runtime versionLock pnpm version for reproducible CI.
- name: Install pnpm - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + with: + version: 9.github/workflows/sonarqube.yml (2)
13-40: Switch to SonarCloud action and gate forked PRs (or add SONAR_HOST_URL for SonarQube)Current setup mixes SonarCloud properties with SonarQube action and lacks fork-PR guard for secrets.
Option A (SonarCloud, recommended):
jobs: sonarqube: name: SonarQube runs-on: ubuntu-latest + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} @@ - - name: SonarQube Scan - uses: SonarSource/sonarqube-scan-action@2500896589ef8f7247069a56136f8dc177c27ccf + - name: SonarCloud Scan + uses: SonarSource/sonarcloud-github-action@v2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}Option B (Self-hosted SonarQube):
jobs: sonarqube: name: SonarQube runs-on: ubuntu-latest + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} @@ - name: SonarQube Scan uses: SonarSource/sonarqube-scan-action@2500896589ef8f7247069a56136f8dc177c27ccf env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }}
32-35: Verify coverage report path
sonar.javascript.lcov.reportPathsis set topackages/js/coverage/lcov.info; runpnpm test:coverageand confirm thelcov.infois generated there (or update the path/config if it’s emitted elsewhere).
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/js/src/index.test.ts(1 hunks)packages/js/src/lib/load-formbricks.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: pandeymangg
PR: formbricks/js#6
File: .github/workflows/build.yml:14-16
Timestamp: 2025-09-24T16:54:10.943Z
Learning: For the formbricks/js repository, the maintainer prefers to keep GitHub Actions build workflows simple with a single Node.js version rather than using matrix strategies to test multiple Node.js versions.
🧬 Code graph analysis (1)
packages/js/src/lib/load-formbricks.test.ts (1)
packages/js/src/lib/load-formbricks.ts (1)
loadFormbricksToProxy(119-130)
🔇 Additional comments (6)
packages/js/src/index.test.ts (5)
4-6: Good ESM‑safe module mock (hoisted)
Mock is declared before importing SUT; Vitest will hoist it. Looks correct.
12-20: Typed mock extraction and reset look solid
Using vi.mocked and resetting resolved value in beforeEach is clean and reliable.
149-160: Dynamic method forwarding is covered well
Good validation of arbitrary method passthrough via the proxy.
180-202: Concurrency behavior check LGTM
Parallel calls with Promise.all and call count assertions are appropriate.
124-127: Clarify init support
The proxy handler dynamically forwardsinitvialoadFormbricksToProxy— no explicit export needed. Keep this test ifinitremains supported; otherwise remove it.packages/js/src/lib/load-formbricks.test.ts (1)
247-256: Queueing warning verified
Accurately asserts the warn message and queuing behavior pre‑init.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(1 hunks).github/workflows/release.yml(1 hunks)packages/js/src/lib/load-formbricks.test.ts(1 hunks)sonar-project.properties(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-24T16:54:20.602Z
Learnt from: pandeymangg
PR: formbricks/js#6
File: .github/workflows/release.yml:30-34
Timestamp: 2025-09-24T16:54:20.602Z
Learning: In the formbricks/js repository, the user pandeymangg prefers using `cd` commands in GitHub Actions workflow run steps rather than using the `working-directory` attribute, even when it might be considered less idiomatic.
Applied to files:
.github/workflows/release.yml.github/workflows/build.yml
📚 Learning: 2025-09-24T17:04:00.699Z
Learnt from: pandeymangg
PR: formbricks/js#6
File: packages/js/src/lib/load-formbricks.test.ts:96-114
Timestamp: 2025-09-24T17:04:00.699Z
Learning: The formbricks/js project uses happy-dom as the test environment in Vitest configuration, not jsdom. This is configured in packages/js/vite.config.ts and works well for DOM-dependent tests.
Applied to files:
packages/js/src/lib/load-formbricks.test.ts
📚 Learning: 2025-09-24T17:04:00.699Z
Learnt from: pandeymangg
PR: formbricks/js#6
File: packages/js/src/lib/load-formbricks.test.ts:96-114
Timestamp: 2025-09-24T17:04:00.699Z
Learning: The formbricks/js project uses happy-dom as the test environment in Vitest configuration (packages/js/vite.config.ts), not jsdom. This configuration works well for DOM-dependent tests and is faster than jsdom.
Applied to files:
packages/js/src/lib/load-formbricks.test.ts
📚 Learning: 2025-09-24T17:03:14.121Z
Learnt from: pandeymangg
PR: formbricks/js#6
File: packages/js/src/index.test.ts:27-103
Timestamp: 2025-09-24T17:03:14.121Z
Learning: In packages/js/src/index.test.ts, the maintainer prefers individual test functions for proxy-forwarding tests rather than table-driven tests, valuing explicit and independently readable test cases.
Applied to files:
packages/js/src/lib/load-formbricks.test.ts
📚 Learning: 2025-09-24T16:54:10.943Z
Learnt from: pandeymangg
PR: formbricks/js#6
File: .github/workflows/build.yml:14-16
Timestamp: 2025-09-24T16:54:10.943Z
Learning: For the formbricks/js repository, the maintainer prefers to keep GitHub Actions build workflows simple with a single Node.js version rather than using matrix strategies to test multiple Node.js versions.
Applied to files:
.github/workflows/build.yml
🧬 Code graph analysis (1)
packages/js/src/lib/load-formbricks.test.ts (1)
packages/js/src/lib/load-formbricks.ts (1)
loadFormbricksToProxy(119-130)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (8)
.github/workflows/release.yml (1)
48-52: Add a tag vs. package version guard before publishing.Publishing without confirming that the release tag matches
packages/js/package.jsonlets an incorrect artifact hit npm and is painful to roll back. Please fail fast if the versions diverge.- name: Run tests run: pnpm test:coverage + - name: Verify tag matches package version + run: | + cd packages/js + PKG_VERSION=$(node -p "require('./package.json').version") + TAG="${GITHUB_REF##refs/tags/}" + if [ "$PKG_VERSION" != "$TAG" ] && [ "v$PKG_VERSION" != "$TAG" ]; then + echo "Release tag ($TAG) does not match packages/js version ($PKG_VERSION)" + exit 1 + fi + - name: Publish to npm run: | cd packages/js npm publish --access public --provenance env:sonar-project.properties (2)
2-3: Confirm Sonar project key/orgVerify sonar.projectKey and sonar.organization match your SonarCloud project to avoid “project not found” in CI.
16-19: Remove deprecated TS coverage key
sonar.javascript.lcov.reportPaths covers both JavaScript and TypeScript coverage; sonar.typescript.lcov.reportPaths is deprecated.Likely an incorrect or invalid review comment.
packages/js/src/lib/load-formbricks.test.ts (5)
167-183: Also assert no duplicate script injection when already loadedStrengthen the test by asserting the script count remains unchanged.
const before = document.querySelectorAll('script[src*="formbricks"]').length; await loadFormbricksToProxy("setup", setupArgs); const after = document.querySelectorAll('script[src*="formbricks"]').length; expect(after).toBe(before);
227-244: Make console error assertion resilient to minor message changes- expect(consoleSpy).toHaveBeenCalledWith( - "🧱 Formbricks - Error: setup failed", - expect.any(Error) - ); + const [msg, err] = consoleSpy.mock.calls[0]; + expect(String(msg)).toMatch(/🧱 Formbricks - Error: setup failed/i); + expect(err).toBeInstanceOf(Error);
80-87: Use Vitest fake timers instead of overriding global setTimeoutReplace the helper with fake timers to avoid global mutation and double-restore.
-const mockSetTimeoutImmediate = () => { - const originalSetTimeout = globalThis.setTimeout; - vi.spyOn(globalThis, "setTimeout").mockImplementation( - immediateTimeoutCallback - ); - return originalSetTimeout; -}; +// Use vi.useFakeTimers()/vi.useRealTimers() within tests that need timer control.
3-5: Strongly type loadFormbricksToProxy instead of anyUse a type-only import of the function type.
-import { describe, test, expect, vi, beforeEach, afterEach } from "vitest"; +import { describe, test, expect, vi, beforeEach, afterEach } from "vitest"; +type LoadFormbricksToProxyFn = typeof import("./load-formbricks").loadFormbricksToProxy; // We need to import the module after each reset -let loadFormbricksToProxy: any; +let loadFormbricksToProxy: LoadFormbricksToProxyFn;
197-207: Refactor timeout test to fake timers (no manual restore)- const originalSetTimeout = mockSetTimeoutImmediate(); - - await loadFormbricksToProxy("setup", setupArgs); - - expect(consoleSpy).toHaveBeenCalledWith( - "🧱 Formbricks - Error: Failed to load Formbricks SDK" - ); - - // Restore setTimeout - globalThis.setTimeout = originalSetTimeout; + vi.useFakeTimers(); + await loadFormbricksToProxy("setup", setupArgs); + vi.runAllTimers(); + expect(consoleSpy).toHaveBeenCalledWith( + "🧱 Formbricks - Error: Failed to load Formbricks SDK" + ); + vi.useRealTimers();
|
mattinannt
left a comment
mattinannt
left a comment
There was a problem hiding this comment.
looks good now; thanks for making the changes :-)
Adds tests for the js package code and the actions for testing, sonarqube scan analysis and releasing the package