feat(Core): create initial core package by ElijahKotyluk · Pull Request #2 · onyxjs/onyx

ElijahKotyluk · 2025-11-16T05:00:39Z

No description provided.

Copilot

Pull Request Overview

This PR establishes the initial core package by introducing a test framework implementation with classes and interfaces for managing test execution, results, and status tracking.

Key Changes:

Introduces a Test class for executing and tracking individual test cases
Defines TaskStatus enum and related interfaces (TestTask, TestResult) for test lifecycle management
Exports core testing types and classes for use across the package

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/core/src/test.ts

Copilot

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/core/src/suite.ts

packages/core/src/expect.ts

packages/core/src/matchers.ts

packages/core/src/suite.ts

…ting for now.

Copilot

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 8 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/core/src/test.ts

packages/core/src/suite.ts

packages/core/src/matchers.ts

packages/core/src/interface.ts

packages/core/src/runner.ts

packages/core/src/cli.ts

.github/workflows/ci.yml

Copilot

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/core/src/matchers.ts

packages/core/src/utils/deepEqual.ts

packages/core/src/runner.ts

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/core/src/runner.ts

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-27T08:08:07Z

packages/core/tsconfig.build.json


  "compilerOptions": {
-    "outDir": "./dist"
+    "outDir": "./dist",


[nitpick] Trailing comma after the last property in the compilerOptions object is inconsistent with JSON best practices and may cause issues with strict JSON parsers.

Suggested change

"outDir": "./dist",

"outDir": "./dist"

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-27T08:31:53Z

packages/core/src/suite.ts

+        description: `${this.getFullDescription()} > ${test.description}`,
+        status: test.status,
+        error: test.error,
+        durationMs: test.durationMs ?? 0,


The test.durationMs field is initialized to 0 in the Test class constructor, so it will never be undefined. The nullish coalescing operator (?? 0) is unnecessary and suggests a mismatch between the implementation and usage.

Suggested change

durationMs: test.durationMs ?? 0,

durationMs: test.durationMs,

Copilot · 2025-11-27T08:31:54Z

packages/core/src/utils/deepEqual.ts

+    // Order of keys matter
+    for (let i = 0; i < xKeys.length; i++) {
+      if (xKeys[i] !== yKeys[i]) return false;
+    }
+


The comment states 'Order of keys matter', but Object.keys() does not guarantee a stable ordering across all JavaScript engines for all object types. This assumption may lead to false negatives when comparing objects with identical properties in different enumeration orders. Consider documenting this limitation or revising the logic to compare keys as sets.

Suggested change

// Order of keys matter

for (let i = 0; i < xKeys.length; i++) {

if (xKeys[i] !== yKeys[i]) return false;

}

// Compare keys as sets (order does not matter)

for (const key of xKeys) {

if (!yKeys.includes(key)) return false;

}

for (const key of yKeys) {

if (!xKeys.includes(key)) return false;

}

Copilot · 2025-11-27T08:31:55Z

packages/core/src/matchers.ts

+  },
+
+  toEqual<T>(this: MatcherContext, received: T, expected: T) {
+    const pass = JSON.stringify(received) === JSON.stringify(expected);


JSON.stringify does not handle undefined values, functions, symbols, Maps, Sets, or circular references correctly. This will produce incorrect equality results for these types. Consider using deepEqual here or documenting that toEqual only supports JSON-serializable values.

Suggested change

const pass = JSON.stringify(received) === JSON.stringify(expected);

const pass = deepEqual(received, expected);

Copilot · 2025-11-27T08:31:55Z

packages/core/test/test.spec.ts

+describe("Failing Test Suite", () => {
+  it("should fail this test", () => {
+    expect(1 + 1).toBe(3);
+  });


This test suite intentionally includes a failing test, which will cause the test run to fail. If this is intended for testing framework behavior, consider isolating it or adding documentation explaining its purpose. Otherwise, fix the assertion to expect(1 + 1).toBe(2).

Copilot · 2025-11-27T08:31:56Z

tsconfig.base.json

  },
-    "exclude": [
+    "ts-node": {
+    // Tell ts-node CLI to install the --loader automatically, explained below


The comment references 'explained below' but no further explanation is provided. Either add the promised explanation or remove this comment to avoid confusion.

Suggested change

// Tell ts-node CLI to install the --loader automatically, explained below

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated no new comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/core/package.json

Copilot · 2025-11-27T10:02:43Z

packages/core/src/interface.ts

+const describe = (description: string, fn: PromisableFn<void>) => {
+  return _describe(description, fn);
+};
+
+const it = (description: string, fn: PromisableFn<void>) => {
+  return _it(description, fn);
+};
+
+export { describe, it };


[nitpick] These wrapper functions are unnecessary - they simply forward to the internal functions without adding any value. Consider exporting _describe and _it directly as describe and it.

Suggested change

const describe = (description: string, fn: PromisableFn<void>) => {

return _describe(description, fn);

};

const it = (description: string, fn: PromisableFn<void>) => {

return _it(description, fn);

};

export { describe, it };

export { _describe as describe, _it as it };

packages/core/src/utils/deepEqual.ts

$chore(core): remove comments, update pkg version.

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-27T16:33:04Z

packages/core/package.json

 {
-  "name": "@onyxjs/core",
-  "version": "1.0.0",
+  "name": "@onyx.js/core",


Package name inconsistency: the package is named '@onyx.js/core' but the import in onyx.config.ts uses '@onyxjs/core'. Consider using a consistent naming convention, either '@onyx.js/core' or '@onyxjs/core' throughout.

Copilot · 2025-11-27T16:33:04Z

packages/core/onyx.config.ts

@@ -0,0 +1,9 @@
+import { defineConfig } from "@onyxjs/core";


Import path '@onyxjs/core' doesn't match the package name '@onyx.js/core' defined in package.json. Update to use '@onyx.js/core' for consistency.

Suggested change

import { defineConfig } from "@onyxjs/core";

import { defineConfig } from "@onyx.js/core";

Copilot · 2025-11-27T16:33:05Z

packages/core/src/interface.ts

+
+import type { PromisableFn } from "./types";
+
+function _describe(description: string, fn: PromisableFn<void>) {


The fn parameter is typed as PromisableFn (which can return a Promise), but it's called synchronously on line 14 without await. This could cause issues if fn returns a Promise. Either change the type to exclude Promise or handle async execution properly.

chore(Test): add initial test class and interface with run method.

6d209de

Copilot AI review requested due to automatic review settings November 16, 2025 05:00

Copilot AI reviewed Nov 16, 2025

View reviewed changes

packages/core/src/test.ts Outdated Show resolved Hide resolved

packages/core/src/test.ts Outdated Show resolved Hide resolved

packages/core/src/test.ts Outdated Show resolved Hide resolved

packages/core/src/test.ts Outdated Show resolved Hide resolved

chore(Core): initial core implementations.

8cf71e2

Copilot AI review requested due to automatic review settings November 27, 2025 01:58

chore(core): link and clean up.

296b015