feat: allow easy inversion of tests#82
Conversation
metalwarrior665
left a comment
There was a problem hiding this comment.
Just a few small nits.
I think this is a useful feature. I was recently thinking about some complex framework to handle temporarily failing tests but then decided against some magic-based solution. On the other hand, this is simple.
Let's wait for @oklinov 's review who has more experience with vitest.
Btw you can build a beta version of the package from this branch and then test it in your repo.
| vi.stubEnv('ACTOR_BUILDS', JSON.stringify([ACTOR_BUILD])); | ||
|
|
||
| const { testActor } = await import('../../lib/lib.js'); | ||
| testActor('my-actor', 'test name', noop, { fails: true }); |
There was a problem hiding this comment.
I'm not a fan of having the options param after the fn but that would need deeper refactor and some type shenanigans to fix. There is already the retry param so this doesn't make it much worse
There was a problem hiding this comment.
I think it's the correct approach, it mirrors vitest parameters nicely
| const name = `${actorName}: ${testName}`; | ||
| const shouldRun = !!RUN_ALL_PLATFORM_TESTS || config.has(actorName); | ||
| vitestTest.runIf(shouldRun)(name, options, async <TYPE extends TestContext>(context: TYPE) => { | ||
| const vitestTestFn = fails && shouldRun ? vitestTest.fails : vitestTest.runIf(shouldRun); |
There was a problem hiding this comment.
This seems cleaner if it does work
| const vitestTestFn = fails && shouldRun ? vitestTest.fails : vitestTest.runIf(shouldRun); | |
| const vitestTestFn = fails ? vitestTest.fails.runIf(shouldRun) : vitestTest.runIf(shouldRun); |
| * The test passes as long as it keeps failing, and alerts you (by failing) if it unexpectedly starts passing. | ||
| * Use this to keep a sentinel for known regressions without inverting assertions by hand. | ||
| */ | ||
| fails?: boolean; |
There was a problem hiding this comment.
I think shouldFail would be clearer. I know it should read like prose but still :)
ruocco-l
left a comment
There was a problem hiding this comment.
I agree with on changing the condition, other than that looks good and makes a lot of sense, thanks!
| vi.stubEnv('ACTOR_BUILDS', JSON.stringify([ACTOR_BUILD])); | ||
|
|
||
| const { testActor } = await import('../../lib/lib.js'); | ||
| testActor('my-actor', 'test name', noop, { fails: true }); |
There was a problem hiding this comment.
I think it's the correct approach, it mirrors vitest parameters nicely
|
@halvko hey, This is already possible with testActor(
ACTOR_ID,
'Newest sorting works for posts that do not expose it in the UI',
async ({ expect, run }) => {
// … run actor …
expect(results[0]?.profileName).toBe('Leif Andersson');
expect(results[10]?.profileName).toBe('Pål Kvitberg');
},
{ fails: true }, // Facebook stopped honouring REVERSE_CHRONOLOGICAL_UNFILTERED_INTENT_V1
// for posts that don't advertise it. Remove `fails` when it works again.
); |
I'm really confused how I missed that 🤔 - I'll make sure my team knows such there can be a followup if there are issues, or we can begin doing it the nicer way if not. I'll close this for now. |
I would like to easily be able to invert a test. Look at the tests for what this actually does, or if you want a very AI generated description:
Feature request:
failssupport intestActorThe use case
Sometimes a scraper relies on a platform-side behaviour that can silently stop working (e.g. sending an undocumented API intent token that Facebook used to honour but no longer does). When that happens, a test that was asserting correct behaviour starts failing. The desired response is:
The current workaround is to manually invert every assertion with
.not, add a prose comment explaining the inversion, and rename the test with a warning in the title. That's fragile and easy to misread.What Vitest already provides
Vitest has
test.fails(). A test wrapped with it is expected to throw/fail. If it does fail, the suite reports it as passed. If it unexpectedly passes, the suite reports it as failed — exactly the sentinel behaviour we want. The symmetry withtest.skip()/test.todo()makes the intent immediately legible to any Vitest user.Why it can't be used today
testActoris the only public entry point for platform tests, and internally it calls:vitestTest.runIf(condition)returns a plain callable; you can't chain.fails()onto it. So consumers have no way to reachvitestTest.fails.runIf(shouldRun)(…)without bypassingtestActorentirely — which means losing the build-pinning logic, therun()helper, and theannotatecalls.Proposed change
Add an optional
failsflag toActorTestOptions(which already extends Vitest'sTestOptions, so the pattern is familiar):Then in
testActorinlib.ts, swap the test function based on the flag:vitestTest.failsis itself a fullTestAPIobject (same shape asvitestTest), so.runIf()is available on it and the rest of the body is unchanged.What the call-site then looks like
The assertions stay in their natural (positive) form, the inversion is expressed as a single structured option rather than spread across comments and
.notcalls, and any Vitest user reading the test immediately understands the intent.