-
-
Notifications
You must be signed in to change notification settings - Fork 12
merge dev to main #58
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
Conversation
* test: policy + client extension interaction * Update packages/runtime/test/policy/client-extensions.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * remove log --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull Request Overview
This PR merges changes from the dev branch into main and updates dependencies as well as introduces new tests. Key changes include dependency version updates in the workspace configuration and package manifest, along with the addition of extensive tests for query override behavior in the runtime policies.
Reviewed Changes
Copilot reviewed 3 out of 7 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pnpm-workspace.yaml | Updated langium and added langium-cli with version 3.5.0 |
| packages/runtime/test/policy/client-extensions.test.ts | Added new tests for different query override scenarios |
| packages/language/package.json | Changed langium-cli dependency from "~3.3.0" to "catalog:" format |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
WalkthroughThe changes update dependency version specifications for Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant TestClient
participant Plugin
participant Database
TestSuite->>TestClient: Setup with model and policy
TestSuite->>Database: Create raw records (bypassing policy)
TestSuite->>TestClient: Register Plugin (override query)
TestSuite->>TestClient: Invoke query (e.g., findMany)
TestClient->>Plugin: Intercept query (modify/filter/mutate)
Plugin->>TestClient: Return modified query/result
TestClient->>Database: Execute query with overrides
Database-->>TestClient: Return results
TestClient-->>TestSuite: Return filtered/mutated results
TestSuite->>TestSuite: Assert results as expected
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/runtime/test/policy/client-extensions.test.tsOops! Something went wrong! :( ESLint: 9.29.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/runtime/test/policy/client-extensions.test.ts (4)
72-73: Same duplicate assertion pattern - consider documentation.Similar to the previous test, if the duplicate assertion is intentional for idempotency testing, a comment would improve clarity.
107-108: Consistent duplicate assertion pattern across tests.The same pattern of duplicate assertions appears across all tests. Consider adding explanatory comments if this is intentional behavior testing.
142-143: Final duplicate assertion in this test pattern.Maintaining consistency with the previous tests' duplicate assertion pattern.
177-179: Duplicate assertion with different expected value structure.This test uses a different expected value format (
expect.objectContaining) but maintains the same duplicate assertion pattern.
🧹 Nitpick comments (2)
packages/runtime/test/policy/client-extensions.test.ts (2)
37-38: Consider consolidating duplicate assertions or add explanation.The same assertion is repeated twice. If this is intentional for testing idempotency, consider adding a comment to clarify the purpose.
await expect(db.$use(ext).model.findMany()).resolves.toHaveLength(1); -await expect(db.$use(ext).model.findMany()).resolves.toHaveLength(1); +// Test idempotency - second call should return same result +await expect(db.$use(ext).model.findMany()).resolves.toHaveLength(1);
28-32: Consider improving type safety in plugin definitions.While
anytypes are common in test code, consider using more specific types for better type safety and IDE support.-model: { - findMany({ args, query }: any) { +model: { + findMany({ args, query }: { args?: any; query: (args: any) => Promise<any> }) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/language/src/generated/ast.tsis excluded by!**/generated/**packages/language/src/generated/grammar.tsis excluded by!**/generated/**packages/language/src/generated/module.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/language/package.json(1 hunks)packages/runtime/test/policy/client-extensions.test.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/runtime/test/policy/client-extensions.test.ts (1)
packages/runtime/test/policy/utils.ts (1)
createPolicyTestClient(5-13)
🔇 Additional comments (4)
pnpm-workspace.yaml (1)
9-10: LGTM! Coordinated dependency version management.The upgrade to fixed versions
3.5.0for bothlangiumandlangium-cliprovides consistency across the workspace. The addition oflangium-clito the catalog properly supports the protocol change in the package file.packages/language/package.json (1)
54-54: Previous concern resolved - dependency now properly cataloged.The change to
"catalog:"protocol is now correct sincelangium-cli: 3.5.0has been properly added to the workspace catalog inpnpm-workspace.yaml.packages/runtime/test/policy/client-extensions.test.ts (2)
1-4: LGTM! Proper test setup and imports.The imports are correctly structured for a vitest-based test suite with appropriate client extension utilities.
146-180: Excellent coverage of result mutation functionality.This test effectively validates that client extensions can modify query results post-execution, demonstrating the plugin system's flexibility for data transformation scenarios.
Summary by CodeRabbit
Tests
Chores
langiumandlangium-cliin workspace and package configuration files to improve consistency and alignment with other dependencies.