Skip to content

Conversation

Smrtnyk
Copy link
Contributor

@Smrtnyk Smrtnyk commented Apr 7, 2025

this moves observable tests from qunit to vitest
this one is pretty straight forward move

Copy link

codesandbox bot commented Apr 7, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview


expect(
// @ts-expect-error -- private method
foo.__eventListeners['bar:baz'].length,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not really a fan of testing implementation details like private methods and members
but this is just a migration from qunit to vitest
I guess if you want you can think of better approach in the future in case you care about this

Copy link
Member

Choose a reason for hiding this comment

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

There are so many things to do, that having test coverage and early catching unwanted changes is already good enough for me. Some tests written for qunit 10 years ago seem silly now.

@asturur asturur merged commit dce6e1b into fabricjs:master Apr 10, 2025
20 of 22 checks passed
@Smrtnyk Smrtnyk deleted the refactor(tests)-move-observable-from-qunit-to-vitest branch April 11, 2025 07:51
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.

2 participants