-
-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: more concise test helpers and unit tests #33
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
e3b3e87
to
9406145
Compare
bed31d7
to
fc745a8
Compare
- adds tests for issues 2 & 3 - runs unit tests at different viewport sizes - very small refactor to createUnitTest for the sake of consistency
8aa2d83
to
14b4e34
Compare
- firefox has a tendency to hang, which makes testing difficult - safari has some support differences which make initial test creation a bit less convenient I'd like to re-add both these once vitest's browser mode is a bit more stable & most of restyle's tests are passing
14b4e34
to
7a4456f
Compare
It took me a bit of reading through the existing tests to fully understand how these new utilities work. I’m curious about your thoughts on using a more declarative snapshot-based API that would more closely mimic how the library is used in practice while still reducing duplication? Thinking something along these lines if possible: import { test, expect } from 'vitest'
import { render } from 'vitest-browser-react'
test("@scope at-rule", () => {
const Div = styled("div", {
"@scope (.start) to (.end)": {
backgroundColor: "blue",
},
});
const screen = render(
<Div>
<Div />
<div className="start">
<Div />
<div className="end">
<Div>@scope</Div>
</div>
</div>
</Div>
);
expect(screen).toMatchSnapshot(`{/* expected CSS styles here somehow */}`);
}); I’m not entirely sure if Vitest’s browser mode supports snapshots yet, but would this remove the need for things like I definitely see why these abstractions exist, and they seem helpful. That said, I’d like to explore a more intuitive approach for now, and in general, I’d lean toward a bit more verbosity before introducing too many abstractions. |
Thinking about this more, using the BeforecreateUnitTest({
name: 'deeply nesting id selectors',
test: styled('div', {
color: 'red',
'#a': {
color: 'green',
'#b': {
color: 'blue',
},
},
})({
children: (
<>
<div id="a">
<div id="b" />
</div>
<div id="b" />
</>
),
}),
expect: (
<div className="a">
<div id="a">
<div id="b" />
</div>
<div id="b" />
</div>
),
css: css`
.a {
color: red;
#a {
color: green;
#b {
color: blue;
}
}
}
`,
}) Aftertest("deeply nesting id selectors", () => {
const screen = render(
<div
css={{
color: "red",
"#a": {
color: "green",
"#b": {
color: "blue",
},
},
}}
>
<div id="a">
<div id="b" />
</div>
<div id="b" />
</div>,
);
expect(screen).toMatchInlineSnapshot(``)
}); Creating a restyle-specific |
Yeah, I'm actually not a huge fan of the multiple utilities here. although it does make writing tests very easy. As far as snapshot testing in general goes, my biggest concerns are:
Otherwise, there are really two broad kinds of tests we'll be dealing with as far as E2E/unit testing goes. testing specific CSS syntaxes and features.there's generally a lot of these, so I personally would prioritize shorter, easier to write tests with utilities handling the heavy lifting. These are also more likely to be written by maintainers who are familiar with the project. The names (and the docs) kinda suck tho, open to ideas. testing specific CSS issuesbehaviors, edge cases, and things like that. there are generally fewer of these, and aren't really exhaustive, mostly just to prevent regressions on things. These will also be written gradually over time, so I'd prefer a more stable, versatile format even if more verbose. These are also more likely to be written by first-time contributers, since they'll often accompany github issues or pull requests. I do actually love the idea of a declarative api where we can test more intricate behaviors, we'll probably need this at some point anyway. maybe something like this? test("@scope at-rule", () => {
const Div = styled("div", {
"@scope (.start) to (.end)": {
backgroundColor: "blue",
},
});
const screen = render(
<Div>
<Div />
<div className="start">
<Div />
<div className="end">
<Div>@scope</Div>
</div>
</div>
</Div>
);
// looks for the specified tag structure with the specified styles,
// doesn't touch the test's DOM, which would be an improvement over the current utils
expectMatchingStyles(
html`
<div className='a'>
Hello World
</div>
`,
css`
.a { color: red }
`
)
}); I'd be pretty happy if something like that replaced |
as I was playing around with the
createUnitTest
helper I noticed when testing basic syntaxes I was repeating myself a lot.I've got duplicate html in test and expect, as well as duplicate css as both an object and string, and for nesting tests I've been testing 3 separate behaviors. and it multiplies, so you for simple unit tests you end up with thousand line unreadable test files with lots of repetition.
I've made
createSyntaxTest
andcreateNestingTest
to help make tests more concise and readable on this front.createSyntaxTest
createSyntaxTest is a simple wrapper over
createUnitTest
that generates matching html and CSS for your test case. This is particularly useful when your testing a specific syntax, for example a media query:children can also accept a function, if you need to nest a styled element within itself.
createNestedTest
createNestedTest is a targeted utility for testing the behavior of
&
combined with a specific selectorthe above example tests the behavior of
#a
,&#a
and& #a
as one suite.