Skip to content

chore(project-config): Use mem fs for testing#1449

Merged
Tobbe merged 7 commits intomainfrom
tobbe-chore-get-config-unused-arg
Mar 24, 2026
Merged

chore(project-config): Use mem fs for testing#1449
Tobbe merged 7 commits intomainfrom
tobbe-chore-get-config-unused-arg

Conversation

@Tobbe
Copy link
Copy Markdown
Member

@Tobbe Tobbe commented Mar 24, 2026

Speed up tests

Note: This PR was initially meant to add a config file cache. But for now I decided that the risk for subtle bugs caused by a stale cache was not worth the unknown benefit of caching the config

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 24, 2026

Deploy Preview for cedarjs canceled.

Name Link
🔨 Latest commit 40fadea
🔍 Latest deploy log https://app.netlify.com/projects/cedarjs/deploys/69c2896d48c45500087b5988

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 24, 2026

Greptile Summary

This PR speeds up the project-config test suite by replacing on-disk TOML fixture files with an in-memory filesystem (memfs), and simplifies call sites that previously passed an explicit configPath to getConfig / getRawConfig (they now rely on the default path resolution via CEDAR_CWD). The aborted config-cache work was rolled back.

  • config.ts: getConfig simplified — configPath is now optional (undefined passes through to getRawConfig's own default), and the double-wrapping try/catch that previously swallowed and re-threw errors from getRawConfig was correctly removed.
  • config.test.ts: Fixture files deleted; tests now write TOML content directly into a memfs volume keyed to /cedar-app. Env-var cleanup for API_PORT was also added. Missing vol.reset() in afterEach and no coverage for the ENOENT path.
  • dbAuth/api/shared.ts: getPort() refactored to a single try/catch, but this inadvertently broadens the catch to include config parse errors, silently returning 8911 instead of surfacing a malformed cedar.toml.
  • cli-helpers/project.ts: getConfig() called without a redundant explicit path argument.

Confidence Score: 4/5

  • Safe to merge once the broadened catch scope in getPort() is addressed; test improvements are non-blocking.
  • The core change (memfs for tests) is clean and correct. The one concrete behavioral regression is in dbAuth/shared.ts where parse errors in cedar.toml are now silently swallowed and return a default port — this affects only the development-environment error-reporting path, not serverless correctness, so the impact is limited. The two test gaps (missing vol.reset() and absent ENOENT test) are best-practice issues rather than functional bugs.
  • packages/auth-providers/dbAuth/api/src/shared.ts — broadened catch scope in getPort()

Important Files Changed

Filename Overview
packages/project-config/src/config.ts getConfig simplified: removed the try/catch wrapper (errors are now fully handled inside getRawConfig, avoiding double-wrapping) and dropped configPath default in favour of passing undefined through to getRawConfig. No cache added (intentionally, per PR description).
packages/auth-providers/dbAuth/api/src/shared.ts getPort() simplified to a single try/catch around getConfig(), but this broadens the catch scope beyond the intended "no cedar.toml in serverless" case: a malformed config file now silently returns 8911 instead of surfacing the parse error.
packages/project-config/src/tests/config.test.ts Tests migrated from on-disk fixture files to memfs inline JSON; API_PORT env var cleanup added. Missing vol.reset() in afterEach and the ENOENT error path is no longer explicitly tested.
packages/cli-helpers/src/lib/project.ts updateTomlConfig simplified: getConfig() called without an explicit path (relies on CEDAR_CWD, consistent with how getConfigPath() already resolves the path just above it).
packages/project-config/src/tests/fixtures/redwood.toml Deleted — content inlined into tests using memfs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[caller invokes getConfig] --> B{configPath provided?}
    B -- yes --> C[pass configPath to getRawConfig]
    B -- no --> D[getRawConfig uses default: getConfigPath]
    D --> E{CEDAR_CWD set?}
    E -- yes --> F[findUp cedar.toml from CEDAR_CWD]
    E -- no --> G[findUp cedar.toml from process.cwd]
    F --> H{file found?}
    G --> H
    H -- no --> I[throw: not a Cedar project]
    H -- yes --> J[cache path in getConfigPathCache]
    C --> K[fs.readFileSync configPath]
    J --> K
    K --> L{read success?}
    L -- no --> M[throw: Could not parse ...]
    L -- yes --> N[envInterpolation]
    N --> O[toml.parse]
    O --> P{parse success?}
    P -- no --> M
    P -- yes --> Q[merge DEFAULT_CONFIG with rawConfig]
    Q --> R[return Config]

    style I fill:#f88,color:#000
    style M fill:#f88,color:#000
    style R fill:#8f8,color:#000
Loading

Reviews (2): Last reviewed commit: "tweak jsdoc" | Re-trigger Greptile

@Tobbe Tobbe changed the title Tobbe chore get config unused arg feat(project-config): Add config cache Mar 24, 2026
Comment thread packages/project-config/src/paths.ts Outdated
Comment thread packages/project-config/src/__tests__/config.test.ts
Comment thread packages/project-config/src/config.ts Outdated
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Mar 24, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 40fadea

Command Status Duration Result
nx run-many -t build:pack --exclude create-ceda... ✅ Succeeded 6s View ↗
nx run-many -t build ✅ Succeeded 5s View ↗
nx run-many -t test --minWorkers=1 --maxWorkers=4 ✅ Succeeded 7s View ↗
nx run-many -t test:types ✅ Succeeded 11s View ↗

☁️ Nx Cloud last updated this comment at 2026-03-24 13:13:12 UTC

@github-actions github-actions Bot added this to the next-release milestone Mar 24, 2026
@Tobbe Tobbe changed the title feat(project-config): Add config cache chore(project-config): Use mem fs for testing Mar 24, 2026
@Tobbe
Copy link
Copy Markdown
Member Author

Tobbe commented Mar 24, 2026

@greptileai please do a full review again

Comment thread packages/auth-providers/dbAuth/api/src/shared.ts
@Tobbe Tobbe enabled auto-merge (squash) March 24, 2026 13:01
@Tobbe Tobbe merged commit 9b44d8f into main Mar 24, 2026
42 checks passed
@Tobbe Tobbe deleted the tobbe-chore-get-config-unused-arg branch March 24, 2026 13:12
@github-actions
Copy link
Copy Markdown

The changes in this PR are now available on npm.

Try them out by running yarn cedar upgrade -t 4.0.0-canary.13664

@Tobbe Tobbe modified the milestones: next-release-patch, v3.1.0 Mar 25, 2026
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.

1 participant