Add pragma for feature testing: @gate - #18581

acdlite · 2020-04-12T04:26:44Z

Alternative to #18574

The @gate pragma declares under which conditions a test is expected to pass.

If the gate condition passes, then the test runs normally (same as if there were no pragma).

If the conditional fails, then the test is expected to fail. This ensures that all tests run in all environments and reduces the likelihood of a test being completely skipped.

An alternative to it.experimental and similar proposals.

Examples

Basic:

// @gate enableBlocksAPI
test('passes only if Blocks API is available', () => {/*...*/})

Negation:

// @gate !disableLegacyContext
test('depends on a deprecated feature', () => {/*...*/})

Multiple flags:

// @gate enableNewReconciler
// @gate experimental
test('only passes in new fork and in experimental channel', () => {/*...*/})

Logical operators (yes, I'm sorry):

// @gate experimental && (enableNewReconciler || disableSchedulerTimeoutBasedOnReactExpirationTime)
test('experimental, doesn\'t work in old fork unless Scheduler timeout flag is disabled', () => {/*...*/})

Strings, and comparison operators

No use case yet but I figure eventually we'd use this to gate on different release channels:

// @gate channel ===  "experimental" || channel === "modern"
test('works in OSS experimental or www modern', () => {/*...*/})

How does it work?

The output of the transform is pretty straightforward:

Input:

// @gate a && (b || c)
test('some test', () => {/*...*/})

Output:

_test_gate(ctx => ctx.a && (ctx.b || ctx.c), 'some test', () => {/*...*/});

It also works with it, it.only, and fit. It leaves it.skip and xit alone because those tests are disabled anyway.

_test_gate is a global method that I set up in our Jest config. It works about the same as the existing it.experimental helper.

The context (ctx) argument is whatever we want it to be. I set it up so that it throws if you try to access a flag that doesn't exist. I also added some shortcuts for common gating conditions, like experimental and stable; and old and new:

// @gate experimental
test('experimental feature', () => {/*...*/})

// @gate stable
test('stable feature, deprecated in experimental', () => {/*...*/})

// @gate old
test('only passes in old reconciler', () => {/*...*/})

// @gate new
test('only passes in new reconciler', () => {/*...*/})

Why implement this as a pragma instead of a dynamic API?

Doesn't require monkey patching built-in Jest methods. Instead it compiles to a runtime function that composes Jest's API.
Will be easy to upgrade if Jest ever overhauls their API or we switch to a different testing framework (unlikely but who knows).
It feels lightweight so hopefully people won't feel gross using it. For example, adding or removing a gate pragma will never affect the indentation of the test, unlike if you wrapped the test in a conditional block.

codesandbox-ci · 2020-04-12T04:28:20Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3b58bb6:

Sandbox	Source
fancy-morning-8h0nc	Configuration

sizebot · 2020-04-12T04:31:30Z

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 3b58bb6

sizebot · 2020-04-12T04:36:36Z

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 3b58bb6

The `@gate` pragma declares under which conditions a test is expected to pass. If the gate condition passes, then the test runs normally (same as if there were no pragma). If the conditional fails, then the test runs and is *expected to fail*. An alternative to `it.experimental` and similar proposals. Examples -------- Basic: ```js // @gate enableBlocksAPI test('passes only if Blocks API is available', () => {/*...*/}) ``` Negation: ```js // @gate !disableLegacyContext test('depends on a deprecated feature', () => {/*...*/}) ``` Multiple flags: ```js // @gate enableNewReconciler // @gate experimental test('needs both useEvent and Blocks', () => {/*...*/}) ``` Logical operators (yes, I'm sorry): ```js // @gate experimental && (enableNewReconciler || disableSchedulerTimeoutBasedOnReactExpirationTime) test('concurrent mode, doesn\'t work in old fork unless Scheduler timeout flag is disabled', () => {/*...*/}) ``` Strings, and comparion operators No use case yet but I figure eventually we'd use this to gate on different release channels: ```js // @gate channel === "experimental" || channel === "modern" test('works in OSS experimental or www modern', () => {/*...*/}) ``` How does it work? I'm guessing those last two examples might be controversial. Supporting those cases did require implementing a mini-parser. The output of the transform is very straightforward, though. Input: ```js // @gate a && (b || c) test('some test', () => {/*...*/}) ``` Output: ```js _test_gate(ctx => ctx.a && (ctx.b || ctx.c, 'some test'), () => {/*...*/}); ``` It also works with `it`, `it.only`, and `fit`. It leaves `it.skip` and `xit` alone because those tests are disabled anyway. `_test_gate` is a global method that I set up in our Jest config. It works about the same as the existing `it.experimental` helper. The context (`ctx`) argument is whatever we want it to be. I set it up so that it throws if you try to access a flag that doesn't exist. I also added some shortcuts for common gating conditions, like `old` and `new`: ```js // @gate experimental test('experimental feature', () => {/*...*/}) // @gate new test('only passes in new reconciler', () => {/*...*/}) ``` Why implement this as a pragma instead of a runtime API? - Doesn't require monkey patching built-in Jest methods. Instead it compiles to a runtime function that composes Jest's API. - Will be easy to upgrade if Jest ever overhauls their API or we switch to a different testing framework (unlikely but who knows). - It feels lightweight so hopefully people won't feel gross using it. For example, adding or removing a gate pragma will never affect the indentation of the test, unlike if you wrapped the test in a conditional block.

acdlite · 2020-04-12T04:56:05Z

I should maybe just swap out the tiny parser I wrote for actual JavaScript. I started out with a more restrictive syntax with ANDs and ORs but then eventually said fuck it and copied JS.

acdlite · 2020-04-12T06:05:32Z

Ahaha forgot to give this a name

Mathspy · 2020-04-12T07:06:22Z

Sorry to barge in! I am curious if this was inspired by a very similar feature in another language. I know Sebastian mentioned taking inspiration from Rust in few occasions recently so I am wondering if you or the whole team are in the same boat
Thanks for this PR it inspired me to make something of the same vein ❤️ And thanks for continuous lovely work on React ❤️

acdlite · 2020-04-12T20:00:34Z

@Mathspy I'm glad you find it interesting!

I am curious if this was inspired by a very similar feature in another language

Hmm I'm sure I was inspired indirectly via exposure but I didn't have a specific feature in mind. It sounds like you're implying Rust has a similar feature. I am decently familiar with Rust but I actually haven't used it much. If you're aware of prior art, please let me know!

Thanks for this PR it inspired me to make something of the same vein

Neat! I was thinking that if we use this for a while and like it, we could try to upstream this to Jest.

We patch console.error and console.warning to track unexpected calls in our tests. If there's an unexpected call, we usually throw inside an `afterEach` hook. However, that's too late for tests that we expect to fail, because our `_test_gate` runtime can't capture the error. So I also check for unexpected calls inside `_test_gate`.

Mathspy · 2020-04-12T23:14:01Z

Oh I see!
For prior art, in Rust there is a specific attribute macro which work pretty much like this, in that it keeps the the module/function/statement that it is placed over or removes it based on a predicate during compilation, here's an example very similar to this:

#[cfg(target_os = "macos")]
fn macos_only() {
 // ...
}

This marks the module with a configuration predicate telling Rust to compile it only for the target of MacOS, otherwise it's completely removed. Another example:

#[cfg(test)]
mod tests {
    // ...
}

This will be compiled only when running in test mode (similar NODE_ENV=test for JS/Node) this allows Rust's unit tests to live in the same file as the units they are testing without being forced to export (mark as public for Rust) some internals or bloating the final binary/bundle

A place where I can see this really shining in JS is actually:

// marks module/function/statement to be conditionally compiled only in debug mode (AKA dev mode; not production)
#[cfg(debug_assertions)]

I think that using a // @dev feature gate in JS would be much more elegant than:

if (__DEV__) {
  // dev mode assertions
}

I am definitely going to try making // @dev and maybe even // @test sometime later this week!

acdlite · 2020-04-12T23:26:11Z

Ah yes I was aware of that feature! I thought you meant more the "negative testing" aspect, where if the test doesn't pass the gating condition, you run it anyway and assert that it fails. That surely has prior art somewhere but I can't think of an example.

Mathspy · 2020-04-12T23:39:52Z

Aha, I definitely share the feeling of "knowing" that specific aspect from somewhere too but just being unable to put my finger on it. If I happen to remember I will definitely leave a comment here!

bvaughn

Didn't read through tokenize very closely. Read the meta tests though, and generally trust you with this sort of change.

bvaughn · 2020-04-13T15:27:27Z

+    throw Error(errorMsg);
  };

+  // TODO: Deprecate these helpers in favor of @gate pragma


Why? What value does a custom pragma offer for iterating on a local focused test?

Oh I meant it.experimental, not the focus helpers. Definitely not removing those!

Ooh, I misunderstood that comment!

That makes sense then.

bvaughn · 2020-04-13T15:34:11Z

+    console.error('Stop that!');
+    throw Error('I told you to stop!');
+  });
+});


Neat testing strategy ^

Added some instructions for how the flags are set up and how to use them.

Receives same flags as the pragma. If we ever decide to revert the pragma, we can codemod them to use this instead.

acdlite · 2020-04-13T17:06:13Z

I'm a little hesitant about the DSL aspect but I'm not too worried because I think in 99% percent of cases, you'll only use a a single flag or a combination of flags, no operators.

So my exit strategy is:

Remove DSL and only support single flags. For more complex cases, add a new flag alias to the TestFlags file.
Or, remove the pragma entirely by codemodding to if statements.

* Add pragma for feature testing: @gate The `@gate` pragma declares under which conditions a test is expected to pass. If the gate condition passes, then the test runs normally (same as if there were no pragma). If the conditional fails, then the test runs and is *expected to fail*. An alternative to `it.experimental` and similar proposals. Examples -------- Basic: ```js // @gate enableBlocksAPI test('passes only if Blocks API is available', () => {/*...*/}) ``` Negation: ```js // @gate !disableLegacyContext test('depends on a deprecated feature', () => {/*...*/}) ``` Multiple flags: ```js // @gate enableNewReconciler // @gate experimental test('needs both useEvent and Blocks', () => {/*...*/}) ``` Logical operators (yes, I'm sorry): ```js // @gate experimental && (enableNewReconciler || disableSchedulerTimeoutBasedOnReactExpirationTime) test('concurrent mode, doesn\'t work in old fork unless Scheduler timeout flag is disabled', () => {/*...*/}) ``` Strings, and comparion operators No use case yet but I figure eventually we'd use this to gate on different release channels: ```js // @gate channel === "experimental" || channel === "modern" test('works in OSS experimental or www modern', () => {/*...*/}) ``` How does it work? I'm guessing those last two examples might be controversial. Supporting those cases did require implementing a mini-parser. The output of the transform is very straightforward, though. Input: ```js // @gate a && (b || c) test('some test', () => {/*...*/}) ``` Output: ```js _test_gate(ctx => ctx.a && (ctx.b || ctx.c, 'some test'), () => {/*...*/}); ``` It also works with `it`, `it.only`, and `fit`. It leaves `it.skip` and `xit` alone because those tests are disabled anyway. `_test_gate` is a global method that I set up in our Jest config. It works about the same as the existing `it.experimental` helper. The context (`ctx`) argument is whatever we want it to be. I set it up so that it throws if you try to access a flag that doesn't exist. I also added some shortcuts for common gating conditions, like `old` and `new`: ```js // @gate experimental test('experimental feature', () => {/*...*/}) // @gate new test('only passes in new reconciler', () => {/*...*/}) ``` Why implement this as a pragma instead of a runtime API? - Doesn't require monkey patching built-in Jest methods. Instead it compiles to a runtime function that composes Jest's API. - Will be easy to upgrade if Jest ever overhauls their API or we switch to a different testing framework (unlikely but who knows). - It feels lightweight so hopefully people won't feel gross using it. For example, adding or removing a gate pragma will never affect the indentation of the test, unlike if you wrapped the test in a conditional block. * Compatibility with console error/warning tracking We patch console.error and console.warning to track unexpected calls in our tests. If there's an unexpected call, we usually throw inside an `afterEach` hook. However, that's too late for tests that we expect to fail, because our `_test_gate` runtime can't capture the error. So I also check for unexpected calls inside `_test_gate`. * Move test flags to dedicated file Added some instructions for how the flags are set up and how to use them. * Add dynamic version of gate API Receives same flags as the pragma. If we ever decide to revert the pragma, we can codemod them to use this instead.

acdlite requested review from bvaughn, gaearon and sebmarkbage April 12, 2020 04:26

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 12, 2020

acdlite mentioned this pull request Apr 12, 2020

Add test helpers for gating by feature #18574

Closed

acdlite force-pushed the gate-pragma branch from 8c0ef7a to a38ef25 Compare April 12, 2020 04:31

acdlite mentioned this pull request Apr 12, 2020

Codemod it.experimental to gate pragma #18582

Merged

acdlite force-pushed the gate-pragma branch from a38ef25 to e0b4965 Compare April 12, 2020 05:08

acdlite commented Apr 12, 2020

View reviewed changes

Comment thread scripts/babel/transform-test-gate-pragma.js Outdated

acdlite Apr 12, 2020

Copy link
Copy Markdown

Collaborator Author

Choose a reason for hiding this comment

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

Ahaha forgot to give this a name

acdlite force-pushed the gate-pragma branch 2 times, most recently from e959688 to f2055bd Compare April 12, 2020 23:09

acdlite force-pushed the gate-pragma branch 2 times, most recently from c487127 to 4f4421c Compare April 12, 2020 23:22

acdlite force-pushed the gate-pragma branch from 4f4421c to 316438f Compare April 13, 2020 06:28

bvaughn approved these changes Apr 13, 2020

View reviewed changes

acdlite added 2 commits April 13, 2020 09:46

Move test flags to dedicated file

7c3bca0

Added some instructions for how the flags are set up and how to use them.

Add dynamic version of gate API

3b58bb6

Receives same flags as the pragma. If we ever decide to revert the pragma, we can codemod them to use this instead.

acdlite merged commit 42d7c2e into react:master Apr 13, 2020

bvaughn mentioned this pull request Aug 5, 2021

Scheduling profiler: Added lane labels and durations to React measures #22029

Merged

4 tasks

Uh oh!

Conversation

acdlite commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Alternative to #18574

Examples

Basic:

Negation:

Multiple flags:

Logical operators (yes, I'm sorry):

Strings, and comparison operators

How does it work?

Why implement this as a pragma instead of a dynamic API?

Uh oh!

codesandbox-ci Bot commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sizebot commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size changes (experimental)

Uh oh!

sizebot commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size changes (stable)

Uh oh!

acdlite commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acdlite Apr 12, 2020

Choose a reason for hiding this comment

Uh oh!

Mathspy commented Apr 12, 2020

Uh oh!

acdlite commented Apr 12, 2020

Uh oh!

Mathspy commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acdlite commented Apr 12, 2020

Uh oh!

Mathspy commented Apr 12, 2020

Uh oh!

bvaughn left a comment

Choose a reason for hiding this comment

Uh oh!

bvaughn Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

acdlite Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

bvaughn Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

bvaughn Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

acdlite commented Apr 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

acdlite commented Apr 12, 2020 •

edited

Loading

codesandbox-ci Bot commented Apr 12, 2020 •

edited

Loading

sizebot commented Apr 12, 2020 •

edited

Loading

sizebot commented Apr 12, 2020 •

edited

Loading

acdlite commented Apr 12, 2020 •

edited

Loading

Mathspy commented Apr 12, 2020 •

edited

Loading