Skip to content
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

Feature: Add a mode to watch with the ability to update an expected literal #4442

Closed
orta opened this issue Sep 7, 2017 · 11 comments
Closed

Comments

@orta
Copy link
Member

orta commented Sep 7, 2017

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

it("is an incorrect test", () => {
  expect(4).toEqual(5)
})

With yarn jest -- --watch

 FAIL  src/_tests/_wrong.test.tsx
  ● is an incorrect test

    expect(received).toEqual(expected)

    Expected value to equal:
      5
    Received:
      4

      at Object.<anonymous>.it (src/_tests/_wrong.test.tsx:2:13)

  ✕ is an incorrect test (94ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        1.147s
Ran all test suites related to changed files.

Watch Usage
 › Press a to run all tests.
 › Press p to filter by a filename regex pattern.
 › Press t to filter by a test name regex pattern.
 › Press q to quit watch mode.
 › Press Enter to trigger a test run.

What is the potential behavior?

What if there was an extra watch mode:

Watch Usage
 › Press a to run all tests.
 › Press u to re-run tests and update failed number literals.
 › Press p to filter by a filename regex pattern.
 › Press t to filter by a test name regex pattern..
 › Press q to quit watch mode.
 › Press Enter to trigger a test run.

Where after pressing u it re-runs with whatever lasts triggered the test run ( so maybe the watchman announcement of a file being saved ) and outputs something like:

 UPDATE  src/_tests/_wrong.test.tsx
  ● is an incorrect test

    Expected value to equal:
      5
    Received:
      4

   Updated test code:

      it("is an incorrect test", () => {
--      expect(4).toEqual(5)
++      expect(4).toEqual(4)
      })

      at Object.<anonymous>.it (src/_tests/_wrong.test.tsx:2:13)

  ✕ is an incorrect test (94ms)

Which updates the file src/_tests/_wrong.test.tsx. Messaging to the watcher should have the same feeling as a snapshot update.

Implementation

This could be an incrementally built out feature:

  • v1: number literals: 4 -> 5
  • v2: string literals: "four" -> "five"
  • v3: object literals: { four: true } -> { five: true }

As number literals have the least amount of potential variants ( vs the multiple types of string literals ) that makes sense as a nice place to start.

I had initially wondered if this could be built out as its own plugin, but once I came to the conclusion about the user experience side of it - it really needs work inside the watcher, though it may be feasible to have the work of AST transformation happen inside a new package ( which could make to easy for dev tools to use it too)

Jest could pass the AST of the it block that failed to the new package, and then it's the job of the package to read through the AST and make the transformation. I don't know if there's an easy way to go from an AST literal back to the LOC/character index (As I've not done too much there) but if prettier/codemods can do it, so can this package.

@orta
Copy link
Member Author

orta commented Sep 7, 2017

There's some interesting feedback against this on twitter:

I can see it being useful if you're absolutely certain about the effect and tests aren't in any way flaky

People changing shared code that breaks tests they are unfamiliar with and running this.

Agreed, if a specific value is being checked there should be friction and thought to update it

If the value may change and should be updated pretty easily, you can use a snapshot

@orta
Copy link
Member Author

orta commented Sep 7, 2017

I'm not sold on these being arguments alone against having a feature like this, there are a bazillion use cases where you need to make small changes to existing expectations - literals like numbers/strings aren't exactly a good match for using snapshots on a larger scale. I get where they're coming from though 👍

Making it more thoughtful is a great idea, so I'd say that you should probably need to confirm each transition in the run:

 FAIL  src/_tests/_wrong.test.tsx
  ● is an incorrect test

    Expected value to equal:
      5
    Received:
      4

   Updated test code:

      it("is an incorrect test", () => {
--      expect(4).toEqual(5)
++      expect(4).toEqual(4)
      })

      in file src/_tests/_wrong.test.tsx:2:13

   Do you want to update? [Yn] 

Which is a better feeling overall for me. Doing so would mean having to re-run in only one process (to allow test running to be blocked) but I think that's a fine trade-off. You're not going to be using this all the time.

One of my end goals will be to allow the jest-editor-support to be able to state "here's something fixable" so that you get this kind of popover (but for changing the string "bu" to "bug")

screen shot 2017-09-07 at 10 07 03

@orta
Copy link
Member Author

orta commented Jan 21, 2018

Mainly so that I can keep track, but VS Code is going to introduce an API allowing extensions to describe changes like this, so editor integration will be pretty simple too - microsoft/vscode#34664

@orta
Copy link
Member Author

orta commented Mar 2, 2018

Got an hour to have a think about this, I think it can be done by allowing matchers to define fixits.

So, the return value from any matcher, can go from:

export type ExpectationResult = {
  pass: boolean,
  message: () => string,
};

to

export type ExpectationResult = {
  pass: boolean,
  message: () => string,
  fixit?: ExpectationFixit,
};

export type ExpectationFixit = {
  message: string,
  type: 'number', // This will get built out as more types are supported
  from: {
    start?: Callsite,
    end?: Callsite,
    value: string,
  },
  to: {
    value: string,
  },
};

So for example:

    toEqual(received: any, expected: any) {
    // [...]
 
    let fixit: ?ExpectationFixit = null;
    if (areBothNumbers(expected, received)) {
      fixit = ExpectationFixit = {
        from: {
          value: expected,
        },
        message: 'Update number to',
        to: {
          value: received,
        },
        type: 'number',
      };
    }

    // Passing the the actual and expected objects so that a custom reporter
    // could access them, for example in order to display a custom visual diff,
    // or create a different error message
    return {actual: received, expected, message, name: 'toEqual', pass, fixit};
  },

Which gets passed up to the error eventually thrown by the matcher. This then gets picked up by something which has to do source mapping somewhere and that can be used to set up the example callsites.

Then a watch plugin can listen out for errors with fixits, or jest-editor-support can ensure it gets passed through JSON and present a UI

@tryggvigy
Copy link
Contributor

tryggvigy commented Mar 15, 2018

Love the proposal! I've got two quick questions @orta

  • › Press u to re-run tests and update failed number literals. This shares the same key used for updating failing snapshots. Do you see Jest fixing literals and snapshots at the same time?

  • Press u to ... What would make sense to say in the case of failing snapshots in the test run as well?

@orta
Copy link
Member Author

orta commented Mar 16, 2018

Do you see Jest fixing literals and snapshots at the same time?

Yeah, maybe it would need a different keyboard shortcut TBH, I would rather they be different personally. I can see a few common use cases where you want to update either separately. Maybe "f" if it's free?

@SimenB
Copy link
Member

SimenB commented Mar 16, 2018

f is for failing tests, ATM

@SimenB SimenB mentioned this issue Jul 19, 2018
10 tasks
@orta
Copy link
Member Author

orta commented Mar 21, 2019

f is for paying respects @SimenB

@tryggvigy
Copy link
Contributor

tryggvigy commented Mar 21, 2019

One way to get away from keyboard shortcuts could be adding a command mode. ++p

Watch Usage
 › Press a to run all tests.
 ...
 › Press ⇧+⌘+p to run a command.
 › Press Enter to trigger a test run.

and it would look similar to filter mode.

Command Mode Usage
 › Press Esc to exit command mode.
 › Press Enter to run a command.

 command › Update Number Literals

Jest itself—or a plugin, could do typeahead and filter the commands in a list below the command prompt similar to what https://github.com/jest-community/jest-watch-typeahead does for watch mode

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 26, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants