Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions projects/cli/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,21 @@ describe('index', () => {

describe('interactive fallback for missing required args', () => {
it('should not exit with validation error for project.create without <type>', () => {
const output = runWithoutRequiredArgs('project.create');
expect(output).not.toContain('Not enough non-option arguments');
expect(output).not.toContain('Missing required argument');
const result = runWithoutRequiredArgs('project.create');
expect(result).not.toContain('Not enough non-option arguments');
expect(result).not.toContain('Missing required argument');
});

it('should not exit with validation error for api.get without <names>', () => {
const output = runWithoutRequiredArgs('api.get');
expect(output).not.toContain('Not enough non-option arguments');
expect(output).not.toContain('Missing required argument');
const result = runWithoutRequiredArgs('api.get');
expect(result).not.toContain('Not enough non-option arguments');
expect(result).not.toContain('Missing required argument');
});

it('should not exit with validation error for project.validate without <type>', () => {
const output = runWithoutRequiredArgs('project.validate');
expect(output).not.toContain('Not enough non-option arguments');
expect(output).not.toContain('Missing required argument');
const result = runWithoutRequiredArgs('project.validate');
expect(result).not.toContain('Not enough non-option arguments');
expect(result).not.toContain('Missing required argument');
});
});

Expand All @@ -102,10 +102,10 @@ describe('index', () => {
encoding: 'utf-8',
input: ''
});
const output = `${result.stdout}${result.stderr}`;
expect(output).not.toContain('"nve-foo,nve-bar"');
expect(output).toContain('nve-foo');
expect(output).toContain('nve-bar');
const combined = `${result.stdout}${result.stderr}`;
expect(combined).not.toContain('"nve-foo,nve-bar"');
expect(combined).toContain('nve-foo');
expect(combined).toContain('nve-bar');
});
});
});
2 changes: 2 additions & 0 deletions projects/cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ yargsInstance.command(

tools
.filter(tool => tool.metadata.support & ToolSupport.CLI)
// eslint-disable-next-line max-lines-per-function
.forEach(tool => {
const { inputSchema, summary } = tool.metadata;
const { properties, required } = inputSchema ?? {};
Expand Down Expand Up @@ -101,6 +102,7 @@ tools
optionalArgs.forEach(key => builder.option(key, argOptions(properties[key]!)));
},
// main handler for the command
// eslint-disable-next-line max-statements
async args => {
const start = performance.now();
const { result, status, message } = await runAsyncTool(args, tool);
Expand Down
74 changes: 36 additions & 38 deletions projects/cli/src/mcp/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';

const { mockRegisterTool, mockRegisterPrompt, mockConnect, mcpTool, cliTool, allTool, mockPrompt, mockZodSchema } =
vi.hoisted(() => {
const mockRegisterTool = vi.fn();
const mockRegisterPrompt = vi.fn();
const mockConnect = vi.fn().mockResolvedValue(undefined);
const mockZodSchema = { shape: {}, optional: () => mockZodSchema };
const zodSchema = { shape: {}, optional: () => zodSchema };

const createMockTool = (overrides: Record<string, unknown>) => {
const fn = vi.fn().mockResolvedValue({ status: 'complete', result: 'test' });
Expand All @@ -27,41 +24,42 @@ const { mockRegisterTool, mockRegisterPrompt, mockConnect, mcpTool, cliTool, all
return fn;
};

const mcpTool = createMockTool({
support: 1,
toolName: 'mcp_tool',
command: 'mcp.tool',
title: 'MCP Tool',
summary: 'MCP only',
inputSchema: undefined
});

const cliTool = createMockTool({
support: 2,
toolName: 'cli_tool',
command: 'cli_tool',
title: 'CLI Tool',
summary: 'CLI only'
});

const allTool = createMockTool({
support: 3,
toolName: 'all_tool',
command: 'all.tool',
title: 'All Tool',
summary: 'All support',
outputSchema: { type: 'string' }
});

const mockPrompt = {
name: 'test-prompt',
title: 'Test Prompt',
description: 'A test prompt',
argsSchema: { type: 'object', properties: {} },
handler: vi.fn().mockResolvedValue({ messages: [] })
return {
mockRegisterTool: vi.fn(),
mockRegisterPrompt: vi.fn(),
mockConnect: vi.fn().mockResolvedValue(undefined),
mockZodSchema: zodSchema,
mcpTool: createMockTool({
support: 1,
toolName: 'mcp_tool',
command: 'mcp.tool',
title: 'MCP Tool',
summary: 'MCP only',
inputSchema: undefined
}),
cliTool: createMockTool({
support: 2,
toolName: 'cli_tool',
command: 'cli_tool',
title: 'CLI Tool',
summary: 'CLI only'
}),
allTool: createMockTool({
support: 3,
toolName: 'all_tool',
command: 'all.tool',
title: 'All Tool',
summary: 'All support',
outputSchema: { type: 'string' }
}),
mockPrompt: {
name: 'test-prompt',
title: 'Test Prompt',
description: 'A test prompt',
argsSchema: { type: 'object', properties: {} },
handler: vi.fn().mockResolvedValue({ messages: [] })
}
};

return { mockRegisterTool, mockRegisterPrompt, mockConnect, mcpTool, cliTool, allTool, mockPrompt, mockZodSchema };
});

vi.mock('@modelcontextprotocol/sdk/server/mcp.js', () => ({
Expand Down
1 change: 1 addition & 0 deletions projects/cli/src/mcp/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { type ToolAnnotations } from '@modelcontextprotocol/sdk/types.js';
export const VERSION = '0.0.0';
export const BUILD_SHA = '__NVE_BUILD_CHECKSUM__';

// eslint-disable-next-line max-lines-per-function
export async function startMcpServer() {
process.env.ELEMENTS_ENV = 'mcp';

Expand Down
6 changes: 3 additions & 3 deletions projects/cli/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ export function getSelect(value: string, prop: { enum?: string[] }) {
return select({
message: `Select a ${value}:`,
choices:
prop.enum?.map(value => ({
name: value,
value
prop.enum?.map(option => ({
name: option,
value: option
})) ?? []
});
}
Expand Down
5 changes: 3 additions & 2 deletions projects/code/src/codeblock/codeblock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,12 @@ export class CodeBlock extends LitElement implements ContainerElement {
const lines = this.formattedCode.split('\n').map((line, index) => {
let span = `${line}`;
if (linesToHighlight.includes(index + 1)) {
let wrapped = line;
// fix for highlightjs multi-line comments
if (line.includes('hljs-comment') && !line.endsWith('</span>')) {
line = `${line}</span>`;
wrapped = `${line}</span>`;
}
span = `<span class="hljs-highlight">${line}</span>`;
span = `<span class="hljs-highlight">${wrapped}</span>`;
}
return span;
});
Expand Down
4 changes: 2 additions & 2 deletions projects/core/build/icons.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ export interface IconSVG {
svg: () => Promise<string> | string;
}

function iconImport(iconImport: () => Promise<{default: string}>): IconSVG {
function iconImport(importer: () => Promise<{default: string}>): IconSVG {
return {
async svg() {
return (await iconImport()).default;
return (await importer()).default;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion projects/core/src/badge/badge.css
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
--border: 0;
--border-radius: var(--nve-ref-border-radius-sm);
--font-weight: var(--nve-ref-font-weight-semibold);
--text-transform: capitalize;
--text-transform: initial;
--width: initial;
--height: var(--nve-ref-size-600);
display: inline-flex;
Expand Down
1 change: 1 addition & 0 deletions projects/core/src/breadcrumb/breadcrumb.css
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ li {
display: flex;
align-items: center;
justify-content: center;
text-wrap: nowrap;
height: var(--_height);
}

Expand Down
2 changes: 1 addition & 1 deletion projects/core/src/card/card-content.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/* SPDX-License-Identifier: Apache-2.0 */

:host {
--padding: var(--nve-ref-size-300) var(--nve-ref-size-400);
--padding: var(--nve-ref-size-400);
padding: var(--padding);
color: var(--color);
flex-grow: 1;
Expand Down
3 changes: 1 addition & 2 deletions projects/core/src/card/card-header.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/* SPDX-License-Identifier: Apache-2.0 */

:host {
--padding: var(--nve-ref-size-300) var(--nve-ref-size-400);
--padding: var(--nve-ref-size-400);
--border-bottom: var(--nve-ref-border-width-sm) solid var(--nve-ref-border-color-muted);
--line-height: 1em;
--gap: var(--nve-ref-space-xs);
Expand All @@ -27,7 +27,6 @@
}

::slotted([slot='title']) {
margin: 0;
font-size: var(--nve-ref-font-size-200);
font-weight: var(--nve-ref-font-weight-bold);
}
Expand Down
10 changes: 5 additions & 5 deletions projects/core/src/color/color.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ describe(Color.metadata.tag, () => {
});

it('should not apply default if custom default is provided', async () => {
const element = document.createElement(Color.metadata.tag) as Color;
const customElement = document.createElement(Color.metadata.tag) as Color;
const input = document.createElement('input');
input.value = '#fff';

element.appendChild(input);
document.body.appendChild(element);
await element.updateComplete;
customElement.appendChild(input);
document.body.appendChild(customElement);
await customElement.updateComplete;
expect(input.value).toBe('#fff');
element.remove();
customElement.remove();
Comment on lines +49 to +57
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Use fixture helpers and guaranteed cleanup for this test path.

At Line 54, attaching directly to document.body bypasses the test fixture pattern, and cleanup at Line 57 is skipped if the test fails early. Prefer createFixture/removeFixture and elementIsStable, with try/finally for deterministic teardown.

Proposed refactor
   it('should not apply default if custom default is provided', async () => {
-    const customElement = document.createElement(Color.metadata.tag) as Color;
-    const input = document.createElement('input');
-    input.value = '#fff';
-
-    customElement.appendChild(input);
-    document.body.appendChild(customElement);
-    await customElement.updateComplete;
-    expect(input.value).toBe('#fff');
-    customElement.remove();
+    const customFixture = await createFixture(html`
+      <nve-color>
+        <input value="#fff" />
+      </nve-color>
+    `);
+    const customElement = customFixture.querySelector(Color.metadata.tag) as Color;
+    const input = customElement.querySelector('input') as HTMLInputElement;
+
+    try {
+      await elementIsStable(customElement);
+      expect(input.value).toBe('#fff');
+    } finally {
+      removeFixture(customFixture);
+    }
   });

As per coding guidelines, projects/core/src/**/*.test.ts: “use createFixture and elementIsStable patterns”.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const customElement = document.createElement(Color.metadata.tag) as Color;
const input = document.createElement('input');
input.value = '#fff';
element.appendChild(input);
document.body.appendChild(element);
await element.updateComplete;
customElement.appendChild(input);
document.body.appendChild(customElement);
await customElement.updateComplete;
expect(input.value).toBe('#fff');
element.remove();
customElement.remove();
const customFixture = await createFixture(html`
<nve-color>
<input value="#fff" />
</nve-color>
`);
const customElement = customFixture.querySelector(Color.metadata.tag) as Color;
const input = customElement.querySelector('input') as HTMLInputElement;
try {
await elementIsStable(customElement);
expect(input.value).toBe('#fff');
} finally {
removeFixture(customFixture);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@projects/core/src/color/color.test.ts` around lines 49 - 57, The test creates
a Color element by hand and appends it to document.body, which bypasses fixture
helpers and risks leaking state if the test fails; replace direct DOM ops with
the project's fixture utilities: use createFixture to instantiate the element
with Color.metadata.tag, await elementIsStable (or the provided elementIsStable
helper) instead of customElement.updateComplete, and wrap assertions in
try/finally calling removeFixture to guarantee teardown; reference the Color
metadata via Color.metadata.tag and the helpers createFixture, removeFixture,
and elementIsStable to make the change.

});

it('should update the color value if EyeDropper is used', async () => {
Expand Down
1 change: 0 additions & 1 deletion projects/core/src/combobox/combobox.examples.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,6 @@ const schema = {
class ComboboxDemo extends LitElement {
@state() private value = [{ name: '', value: '' }];

// todo
/* eslint-disable @nvidia-elements/lint/no-deprecated-popover-attributes */
render() {
return html`
Expand Down
45 changes: 22 additions & 23 deletions projects/core/src/combobox/combobox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ describe(Combobox.metadata.tag, () => {
it('should reflect each option to a menu item', async () => {
emulateClick(input);
await elementIsStable(element);
const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
expect(items.length).toBe(3);
expect(items[0].textContent.trim()).toBe(options[0].value);
expect(items[1].textContent.trim()).toBe(options[1].value);
expect(items[2].textContent.trim()).toBe(options[2].value);
const menuItems = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
expect(menuItems.length).toBe(3);
expect(menuItems[0].textContent.trim()).toBe(options[0].value);
expect(menuItems[1].textContent.trim()).toBe(options[1].value);
expect(menuItems[2].textContent.trim()).toBe(options[2].value);
});

it('should default the dropdown to align bottom center with position-area "bottom span-right" ensure wide selection options span to the right of the input', async () => {
Expand Down Expand Up @@ -107,10 +107,10 @@ describe(Combobox.metadata.tag, () => {
it('should each menu item with the aria role of option', async () => {
emulateClick(input);
await elementIsStable(element);
const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
expect(items[0].getAttribute('role')).toBe('option');
expect(items[1].getAttribute('role')).toBe('option');
expect(items[2].getAttribute('role')).toBe('option');
const menuItems = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
expect(menuItems[0].getAttribute('role')).toBe('option');
expect(menuItems[1].getAttribute('role')).toBe('option');
expect(menuItems[2].getAttribute('role')).toBe('option');
});

it('should only show options that partial match the input value', async () => {
Expand Down Expand Up @@ -147,9 +147,9 @@ describe(Combobox.metadata.tag, () => {
await elementIsStable(element);
expect(dropdown.matches(':popover-open')).toBe(true);

const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
items[0].focus();
items[0].dispatchEvent(new KeyboardEvent('keydown', { code: 'Escape', bubbles: true }));
const menuItems = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
menuItems[0].focus();
menuItems[0].dispatchEvent(new KeyboardEvent('keydown', { code: 'Escape', bubbles: true }));
await elementIsStable(element);
expect(dropdown.matches(':popover-open')).toBe(false);
});
Expand All @@ -164,7 +164,7 @@ describe(Combobox.metadata.tag, () => {
});

it('should focus first option if key arrow down is pressed', async () => {
const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
const menuItems = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
const dropdown = element.shadowRoot.querySelector<Dropdown>(Dropdown.metadata.tag);
expect(dropdown.matches(':popover-open')).toBe(false);
element.dispatchEvent(new KeyboardEvent('keydown'));
Expand All @@ -176,8 +176,8 @@ describe(Combobox.metadata.tag, () => {
input.focus();
await open;
element.dispatchEvent(new KeyboardEvent('keydown', { code: 'ArrowDown' }));
expect(items[0].tabIndex).toBe(0);
expect(element.shadowRoot.activeElement.tagName).toBe(items[0].tagName);
expect(menuItems[0].tabIndex).toBe(0);
expect(element.shadowRoot.activeElement.tagName).toBe(menuItems[0].tagName);
});

it('should hide dropdown on keydown and is a tab event', async () => {
Expand Down Expand Up @@ -209,24 +209,24 @@ describe(Combobox.metadata.tag, () => {
});

it('should set the input value if a option is clicked', async () => {
const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
const menuItems = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
const dropdown = element.shadowRoot.querySelector<Dropdown>(Dropdown.metadata.tag);
expect(dropdown.matches(':popover-open')).toBe(false);

element.dispatchEvent(new KeyboardEvent('keydown'));
await elementIsStable(element);
expect(dropdown.matches(':popover-open')).toBe(true);

emulateClick(items[0]);
emulateClick(menuItems[0]);
expect(input.value).toBe('Option 1');
});

it('should show "no results" message if no options are provided', async () => {
const options = element.querySelectorAll('option');
const allOptions = element.querySelectorAll('option');
const dropdown = element.shadowRoot.querySelector<Dropdown>(Dropdown.metadata.tag);
expect(dropdown.matches(':popover-open')).toBe(false);

options.forEach(i => i.remove());
allOptions.forEach(i => i.remove());
element.shadowRoot.dispatchEvent(new Event('slotchange'));
element.dispatchEvent(new KeyboardEvent('keydown'));
await elementIsStable(element);
Expand Down Expand Up @@ -270,15 +270,14 @@ describe(Combobox.metadata.tag, () => {
});

it('should filter out menu items when corresponding option is disabled', async () => {
const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
expect(items[0].disabled).toBe(false);
const menuItems = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
expect(menuItems[0].disabled).toBe(false);
options[0].disabled = true;
element.shadowRoot.dispatchEvent(new Event('slotchange'));
await elementIsStable(element);
expect(items[0].disabled).toBe(true);
expect(menuItems[0].disabled).toBe(true);
});

// todo: this should be handled better at runtime
it('should throw when selectAll() is called on datalist combobox without a select element', async () => {
expect(() => element.selectAll()).toThrow();
});
Expand Down
1 change: 0 additions & 1 deletion projects/core/src/copy-button/copy-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ export class CopyButton extends Button {
[Tooltip.metadata.tag]: Tooltip
};

// todo
/* eslint-disable @nvidia-elements/lint/no-deprecated-popover-attributes */
render() {
return html`
Expand Down
Loading
Loading