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

feat(stubs): render function props deterministically, fix #975 #1834

Merged
merged 1 commit into from Apr 30, 2021
Merged

feat(stubs): render function props deterministically, fix #975 #1834

merged 1 commit into from Apr 30, 2021

Conversation

MitchLillie
Copy link
Contributor

@MitchLillie MitchLillie commented Apr 28, 2021

Since @briwa 's PR languished and we haven't heard from them, I'm re-opening the same. Tests are working fine for me.

Closes #975 and #1209.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

Applications with shallow mounted snapshot testing of components that pass functions to child components should update their snapshots.

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Code seems fine, but isn't this a major breaking change for shallow mounted snapshots? I am kind of surprised you didn't have to change other tests asserting against wrapper.html().

I could be missing something - it's been a while since I touched this part of the code base.

@MitchLillie
Copy link
Contributor Author

Yes, that's true. This will break any shallow mounted snapshots that pass a function to a child property. So definitely a breaking change, but in our repo with 2601 tests, only four had this problem. If you look at the linked issues, most folks are simply skipping testing or coverage on these problem areas, because the snapshots are completely non-deterministic between environments.

@lmiller1990
Copy link
Member

lmiller1990 commented Apr 30, 2021

Alright, good to know. The next release will be a minor (1.2.0) release in this case. I'll try to do it this week. Thanks!

@lmiller1990 lmiller1990 merged commit 97fdf18 into vuejs:dev Apr 30, 2021
@lmiller1990
Copy link
Member

@MitchLillie it's out. Can you give it a test in your prod code base and let me know if we missed anything, or you have any unexpected breakages?

@MitchLillie
Copy link
Contributor Author

@lmiller1990 Just tested, looks good on our end. We had 7 total snapshots that needed updating, out of 39 total. That's more than the original 4 because 3 were referencing mocked functions, which were already ignored in test coverage, so didn't have this issue.

@MitchLillie MitchLillie deleted the function-props branch May 4, 2021 18:06
@pazderka
Copy link

pazderka commented May 5, 2021

Version 1.2.0 has resolved most of the issues that i have had with rendering snapshots deterministically, however i have still one failing scenario.

I am using Vuetify and the snapshot test fails for the prop allowed-dates which Vuetify DatePicker exposes. This prop seems to produce unexpected results in snapshot tests, the failure looks like this, for example:

                <div
                  allowed_dates="() => {
                  /* istanbul ignore next */
    -             cov_1o1cojjd49().f[12]++;
    -             cov_1o1cojjd49().s[34]++;
    +             cov_kawvohiv4().f[12]++;
    +             cov_kawvohiv4().s[34]++;
                  return true;
                }"
                  class="DatePickerMenu-stub"
                  label="common:created_from"
                />

Any ideas how to get this to work?

@lmiller1990
Copy link
Member

I am not sure why coverage is even showing up in the snapshots. My guess is this needs to be explored in vue-jest probably - not sure there is something to be done here 🤔

I have not much experience with code coverage, but happy to help investigate if you are interested in doing so. Can you reproduce this easily with a fresh Vue cli project + some config?

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.

Function properties in stubs should not render source code
3 participants