Skip to content

chore: Modernize the test suite#84

Merged
rzane merged 29 commits into
mainfrom
rzane/modernize-tests
Jan 2, 2026
Merged

chore: Modernize the test suite#84
rzane merged 29 commits into
mainfrom
rzane/modernize-tests

Conversation

@rzane

@rzane rzane commented Dec 30, 2025

Copy link
Copy Markdown
Contributor

This PR cleans up the test suite in preparation for some larger refactors. I'm not touching any source files or build processes in this PR.

  • Converts tests to TypeScript
  • Switches from Jest to Vitest
  • Replaces axios-mock-adapter with MSW, which will make it easier to remove Axios
  • Replaces testContext pattern.

@rzane rzane force-pushed the rzane/modernize-tests branch from dcf8522 to 54659b2 Compare December 30, 2025 02:17
@rzane rzane marked this pull request as ready for review December 30, 2025 02:45

@samandmoore samandmoore 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.

domainlgtm

const visitor = createVisitor();

expect(() => {
new ABConfiguration({ splitName: 'button_color', visitor: visitor });

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.

is this missing the comment for ts-expect-error?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the types are just wrong here. We should get rid of all of these tests and all of the validations in the constructor. This is all private, so we're effectively validating that our own code doesn't have bugs, and typescript already does that for us in this regard.

splitName: 'element',
trueVariant: 'water',
visitor: testContext.visitor
visitor: visitor

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.

could also use just visitor here like you do below. surprised we don't lint or format into a single approach.

also fine to leave it for now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we need to tighten up the linting here

@samandmoore samandmoore 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.

platformlgtm toooo

@rzane rzane merged commit ca0f35d into main Jan 2, 2026
5 checks passed
@rzane rzane deleted the rzane/modernize-tests branch January 2, 2026 15:17
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