Skip to content

Test linting behavior in integration (close to real world)#82

Merged
barmac merged 4 commits into
mainfrom
lint-emitter
Feb 10, 2026
Merged

Test linting behavior in integration (close to real world)#82
barmac merged 4 commits into
mainfrom
lint-emitter

Conversation

@nikku

@nikku nikku commented Feb 9, 2026

Copy link
Copy Markdown
Member

Proposed Changes

We previously did not test editor linting behavior during value changes. We also did not use the standard "batteries included" by codemirror, but took a (faster) shortcut to force linting. That works "for tests", but does not verify actual user behavior.

This PR adds some real-world tests, to validate linting works as expected across the use-cases we cover.

Checklist

Ensure you provide everything we need to review your contribution:

  • Contribution meets our definition of done
  • Pull request establishes context
    • Link to related issue(s), i.e. Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}
    • Brief textual description of the changes
    • Screenshots or short videos showing UI/UX changes
    • Steps to try out, i.e. using the @bpmn-io/sr tool

Copilot AI review requested due to automatic review settings February 9, 2026 16:00
@bpmn-io-tasks bpmn-io-tasks Bot added the needs review Review pending label Feb 9, 2026
@nikku nikku changed the title Test editor behavior in integration manner Test linting behavior in integration (close to real world) Feb 9, 2026
@nikku nikku force-pushed the lint-emitter branch 2 times, most recently from 8f14f75 to 1442a6c Compare February 9, 2026 16:03
test: ensure we validate linting in real world setup (no mocking)

Copilot AI left a comment

Copy link
Copy Markdown

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 aims to test CodeMirror linting behavior in a more integration-like (“real user”) way, validating that linting triggers correctly on initial render and after value updates.

Changes:

  • Update lint-related tests to assert on actual diagnostics arrays and add integration-style lint trigger tests.
  • Introduce a lint event on FeelEditor (backed by a new mitt event emitter) and wire existing onLint through it.
  • Bump several CodeMirror dependencies, add mitt, and replace karma-chrome-launcher with karma-chrome-launcher-2.

Reviewed changes

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

File Description
test/spec/CodeEditor.spec.js Updates lint assertions to use diagnostics arrays; adds integration-style tests for lint triggering and new lint helpers.
src/index.js Adds an internal event emitter and exposes on/off; emits lint events and forwards them to the existing onLint callback.
package.json Updates dependencies (CodeMirror patches), adds mitt, and switches to karma-chrome-launcher-2.
package-lock.json Locks updated dependency graph reflecting the package.json changes.

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

Comment thread test/spec/CodeEditor.spec.js
Comment thread test/spec/CodeEditor.spec.js
Comment thread src/index.js
Comment thread package.json

@barmac barmac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@barmac barmac merged commit ae7284f into main Feb 10, 2026
7 checks passed
@bpmn-io-tasks bpmn-io-tasks Bot removed the needs review Review pending label Feb 10, 2026
@barmac barmac deleted the lint-emitter branch February 10, 2026 12:46
@barmac

barmac commented Feb 10, 2026

Copy link
Copy Markdown
Member

Released in v2.4.0. I also updated the readme to reflect new API: https://github.com/bpmn-io/feel-editor?tab=readme-ov-file#events

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.

3 participants