Skip to content

Minify content tests, update dependencies#49

Merged
danielsitek merged 7 commits intomasterfrom
feature/ds-minify-content-tests
Dec 9, 2025
Merged

Minify content tests, update dependencies#49
danielsitek merged 7 commits intomasterfrom
feature/ds-minify-content-tests

Conversation

@danielsitek
Copy link
Copy Markdown
Owner

@danielsitek danielsitek commented Dec 9, 2025

This pull request updates the development environment and testing framework to use more recent versions of Node.js and TypeScript, enhances the custom test framework with new assertions, and adds comprehensive tests for the HTML minification logic. The changes improve compatibility, add useful test utilities, and increase test coverage.

Dependency and environment updates:

  • Updated the supported Node.js versions in the GitHub Actions workflow to only include 20, 22, and 24, dropping support for Node.js 18. (.github/workflows/dev.yml)
  • Upgraded the typescript dependency from version 3.9.10 to 5.9.3 in both the root and simple-test-framework package, and updated the minimum Node.js engine requirement to 20.0.0 in the root package.json. (local_modules/simple-test-framework/package.json, local_modules/simple-test-framework/package-lock.json, package.json) [1] [2] [3] [4]

Testing framework improvements:

  • Added new assertion methods toContain and not.toContain to the expect API in simple-test-framework, allowing for more expressive tests involving arrays and strings. (local_modules/simple-test-framework/src/index.ts)

Test coverage:

  • Added a new test suite for the minifyContents function, covering HTML minification, non-HTML passthrough, case and whitespace handling in file types, and robustness against invalid HTML. (src/minify/minifyContents.spec.ts)

Summary by CodeRabbit

  • New Features

    • Added new assertion methods to the test framework: toContain() for validating array and string elements, not.toContain() for negated containment checks, and not.toBe() for inequality assertions.
  • Chores

    • Updated minimum Node.js requirement from 18 to 20.
    • Updated TypeScript and tooling dependencies.
    • Added comprehensive test coverage for content minification.

✏️ Tip: You can customize this high-level summary in your review settings.

@danielsitek danielsitek self-assigned this Dec 9, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 9, 2025

Walkthrough

This PR updates the test framework's TypeScript version to 5.9.3, extends the expect API with toContain and negated assertions, renames findAndRunTests to testsRunner, adds a comprehensive test suite for minifyContents, updates Node.js engine requirement to 20+, and removes Node.js 18 from the CI matrix.

Changes

Cohort / File(s) Summary
CI Configuration
.github/workflows/dev.yml
Removed Node.js version 18 from test matrix, leaving [20, 22, 24]
Test Framework Dependencies
local_modules/simple-test-framework/package.json
Updated TypeScript devDependency from 3.9.10 to 5.9.3
Test Framework API
local_modules/simple-test-framework/src/index.d.ts
local_modules/simple-test-framework/src/index.ts
Extended expect() return type with toContain(), not.toContain(), and not.toBe() assertions; renamed exported function findAndRunTests(dir: string) to const testsRunner: (cwd: string) => Promise<void>; removed declare keyword from TestFunction type alias
Project Dependencies
package.json
Removed @types/html-minifier-next devDependency; added ts-node@10.9.2 devDependency; updated engines.node from >=18.0.0 to >=20.0.0
Test Coverage
src/minify/minifyContents.spec.ts
Added comprehensive test suite for minifyContents covering HTML minification, non-HTML handling, case-insensitivity, whitespace tolerance, and graceful error handling

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify that all call sites of findAndRunTests have been updated to use testsRunner (check bin/run-tests.ts and other entry points)
  • Ensure the new assertion helper implementations (toContain, not.toContain, not.toBe) properly handle edge cases and type safety
  • Confirm the minifyContents test scenarios adequately cover expected behavior with HTML and non-HTML content types

Possibly related PRs

  • CI tests #40: Directly overlaps with this PR's renaming of findAndRunTests to testsRunner and test-framework API enhancements
  • Change html-minify package #48: Shares test coverage focus on minifyContents implementation and behavior
  • Update dependencies #46: Modifies simple-test-framework source alongside this PR's assertion API extensions

Poem

🐰 With TypeScript bright and assertions new,
The test framework dances in version two.
No eighteen here, just twenty and more,
Coverage grows—what's not to adore?
From testsRunner's hop to expect's grand display,
Tests bloom and the rabbits rejoice today!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures two main themes of the changeset: new test coverage for minifyContents and dependency updates (TypeScript, Node versions, new assertion methods).
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/ds-minify-content-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
src/minify/minifyContents.spec.ts (1)

4-60: Good coverage of minifyContents behavior, but some assertions are quite brittle

This suite nicely exercises the key paths: HTML minification, non‑HTML passthrough, fileType normalization (case + whitespace), and robustness to invalid HTML, all aligned with the current minifyContents implementation.

Two minor points you may want to consider:

  • A few expectations tightly couple to the exact formatting of the minifier output (e.g., not.toContain('\n'), exact <div>test</div>, specific <!doctype html> casing). If the underlying minifier changes its formatting without changing semantics, these tests could start failing; relaxing to more semantic checks (e.g., presence of key substrings, or using regex) would make them less fragile.
  • On Line 34, }); test('should return original content if fileType is not html', ...) on the same line is valid but a bit hard to read; splitting the second test onto its own line would improve clarity.
local_modules/simple-test-framework/src/index.ts (1)

168-195: testsRunner is a good public entrypoint; just be aware it always terminates the process

The testsRunner wrapper around findAndRunTests cleanly aggregates failures, prints a summary with timing, and exits with the appropriate status code. That’s ideal for a CLI, but it does mean that any consumer calling testsRunner programmatically won’t regain control afterward. If you ever need a library-style API, you might later want a variant that returns a result object instead of calling process.exit.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 121350c and 2d441b5.

⛔ Files ignored due to path filters (2)
  • local_modules/simple-test-framework/package-lock.json is excluded by !**/package-lock.json
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (6)
  • .github/workflows/dev.yml (1 hunks)
  • local_modules/simple-test-framework/package.json (1 hunks)
  • local_modules/simple-test-framework/src/index.d.ts (2 hunks)
  • local_modules/simple-test-framework/src/index.ts (1 hunks)
  • package.json (1 hunks)
  • src/minify/minifyContents.spec.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
local_modules/simple-test-framework/src/index.d.ts (1)
local_modules/simple-test-framework/src/index.ts (2)
  • runTestFile (143-148)
  • testsRunner (168-195)
src/minify/minifyContents.spec.ts (2)
src/minify/minifyContents.ts (1)
  • minifyContents (12-23)
local_modules/simple-test-framework/src/index.ts (1)
  • expect (90-141)
⏰ 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: Codacy Static Code Analysis
🔇 Additional comments (5)
local_modules/simple-test-framework/package.json (1)

12-15: TypeScript devDependency bump looks consistent with repo tooling

Aligning this package’s TypeScript version with the root (5.9.3) keeps the build/test tooling coherent; no issues from what’s visible here, as long as the existing tsconfig still compiles cleanly under TS 5.9.x.

.github/workflows/dev.yml (1)

19-21: CI Node matrix correctly aligned with engines requirement

Targeting Node 20, 22, and 24 matches the updated "engines": { "node": ">=20.0.0" } and should keep CI focused on supported runtimes. Just ensure your dependencies are confirmed compatible with Node 24 as well, since that’s sometimes ahead of many libraries’ tested ranges.

package.json (1)

61-70: Dev tooling and engines updates are coherent with the rest of the PR

Adding ts-node as a devDependency and bumping "engines.node" to >=20.0.0 line up with your dev scripts and the updated CI matrix. From what’s here, the setup is consistent; just make sure ts-node@10.9.2 runs clean with TypeScript 5.9.x on Node 20+ by exercising the dev:* scripts.

local_modules/simple-test-framework/src/index.ts (1)

90-140: expect() matcher extensions are consistent and type-safe enough for this framework

The new toContain and not matchers correctly handle arrays and strings, provide clear error messages, and match the declared typings. Using a generic T for toBe / not.toBe and a looser any for toContain is a pragmatic choice here given the runtime type checks.

local_modules/simple-test-framework/src/index.d.ts (1)

1-17: Typings correctly mirror the runtime API

The updated declarations for expect (including toContain and not.{toContain,toBe}), runTestFile, and testsRunner all match the implementation in src/index.ts. The internal TestFunction alias and the non-exported findAndRunTests keep the public surface minimal and clear.

@danielsitek danielsitek merged commit e1366bd into master Dec 9, 2025
10 checks passed
@danielsitek danielsitek deleted the feature/ds-minify-content-tests branch December 9, 2025 20:06
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