fix: update VTU to v1, update types #139

afenton90 · 2020-05-06T14:23:07Z

Upgrades version of @vue/test-utils to ^1.0.1.

Replace attachToDocument with attachTo and mirror default behaviour
Remove guard for wrapper.destroy as destroy must always be called now is using attachTo see @vue/test-utils docs here.
~~Adds config.showDeprecationWarnings = false as isVueInstance is still used internally by @vue/test-utils and gives warnings.~~

closes #138

afenton90 · 2020-05-06T14:27:07Z

☝️ test failed because Cannot find module '@babel/compat-data/corejs3-shipped-proposals' doesn't seem to have change in package-lock.json file though.

afontcu · 2020-05-06T14:48:42Z

src/__tests__/vuetify.js

@@ -69,5 +69,5 @@ test('opens a menu', async () => {
  expect(menuItem).toBeInTheDocument()

  await fireEvent.click(menuItem)
-  expect(queryByText('menu item')).not.toBeInTheDocument()
+  expect(queryByText('menu item')).not.toBeVisible()


Hm, did this test start to fail? 🤔 I mean, did Vuetify changed anything? I'm worried this is related to attachTo

Yeah it did start to fail. When I debugged though it seemed like once menu list was open it just added display none to hide it.
Let me debug with attachToDocument to see what it was outputting

So with master this works fine. Using debug shows that the menu item is removed from the DOM.
However, with the latest version of VTU and attachToDocument still show this in the DOM.

So looks like an issue with the latest release of VTU rather than use of attachTo

Ok, I spent some time on this. The breaking change is actually from vue/[email protected] -> vue/[email protected].

I will debug a bit more.

Don't ask how I figured this out but THIS LINE is the problem. Adding it back in fixes this test.

I do not know why this was removed or what this line does.

Hey! 👋 yeah, I'm currently looking into it. I started to think that toBeInTheDocument() was actually not the right assertion.

AFAICT, Vuetify does render the element but hides it, so toBeInTheDocument() should have passed. Now, I don't have much experience with Vuetify, so I might be wrong.

I am unable, though, to make this pass (and I think it should):

test('opens a menu', async () => { const {getByText, queryByText} = renderWithVuetify(VuetifyDemoComponent) const openMenuButton = getByText('open menu') // Menu item is initially not rendered expect(queryByText('menu item')).not.toBeInTheDocument() await fireEvent.click(openMenuButton) const menuItem = getByText('menu item') expect(menuItem).toBeInTheDocument() expect(menuItem).toBeVisible() // <-- this fails (it still has "display: none;") await fireEvent.click(openMenuButton) expect(menuItem).not.toBeVisible() })

@afontcu so just tried the menu in a project I have locally, the browser behaviour is:

Mounted v-menu does not appear in the document

Users clicks activator. In my case a v-text-field. v-menu is inserted into the DOM

User clicks activator again to close the menu. v-menu is still in the DOM but has display: none

So the edit to the test looks valid based on this behaviour.
Looks like the test before was reporting a false positive.

Looks like the test before was reporting a false positive.

Yes! That's what I thought, too 🙌 . The snippet I posted above is currently failing, I'll try to dig a bit more and see if I can figure out why. Thought it could have something to with transitions, even though I tried with <v-menu :transition="false" /> but nothing changed.

Tried a lot of different versions of this test, then figured:

"I wonder how Vuetify are testing the menu component"

Turns out the definition of this "working" is that it renders with display: none even when clicked whilst testing with VTU.

There is a snapshot here with display: none https://github.com/vuetifyjs/vuetify/blob/9ac1158f857fd1f531f1177ba623602973f81b55/packages/vuetify/src/components/VMenu/__tests__/__snapshots__/VMenu.spec.ts.snap#L15
With the corresponding test here https://github.com/vuetifyjs/vuetify/blob/9ac1158f857fd1f531f1177ba623602973f81b55/packages/vuetify/src/components/VMenu/__tests__/VMenu.spec.ts#L32

okay, this is interesting. Now I'm leaning towards simplifying our Vuetify test (which is only aimed at showing a way of configuring VTL and Vuetify) to make it more resilient. If they are happy with display:none, we should be, too. I'm gonna push some changes soon to this branch, and make the test pass.

Thanks for your help, @afenton90! Much appreciated :)

removes config for depracation notice as isVueInstance is now removed from internals

marcpeabody · 2020-05-12T17:28:11Z

Does this still need work before it can be merged? I see the CI failing Error: Cannot find module '@babel/compat-data/corejs3-shipped-proposals'

afontcu · 2020-05-12T17:43:38Z

Does this still need work before it can be merged? I see the CI failing Error: Cannot find module '@babel/compat-data/corejs3-shipped-proposals'

hey! yes, it is still WIP. It's been a busy week and I couldn't move it forward. I'll try to find some time, but happy to see it move forward if anyone's interested!

Things I'd like to see fixed before merging this

Is the failing vuetify.js test an upstream issue with VTU, or it is on Vue Testing Lib? I'm a bit worried it is related with how we're attaching to the container (here).
Fix CI

afenton90 · 2020-05-13T08:48:25Z

@afontcu I tried a few different versions of the upgrade to see if I could figure out why the test has to be changed. Unfortunately, I have no extra information on top of my last comment here #139 (comment) which is my best theory at the moment.

As for fixing of CI it seems to be specific to node 14 build. I can't see anything directly correlated with code changes in the PR.

Any help to get this over the line would be appreciated.

afontcu · 2020-05-13T20:19:36Z

As for fixing of CI it seems to be specific to node 14 build. I can't see anything directly correlated with code changes in the PR.

Looks like it was fixed in kcd-scripts v5.11.1: kentcdodds/kcd-scripts#139. Just pushed a version bump, all green now :)

codecov · 2020-05-13T20:24:06Z

Codecov Report

Merging #139 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #139   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           68        69    +1     
  Branches        14        13    -1     
=========================================
+ Hits            68        69    +1

Impacted Files	Coverage Δ
src/vue-testing-library.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22c34f7...dc38df6. Read the comment docs.

kuddl · 2020-05-15T13:51:43Z

Please merge?? Sounds like it's ready to go ? And our consoles are cluttered with error messages!

afontcu · 2020-05-16T20:10:37Z

Please merge?? Sounds like it's ready to go ? And our consoles are cluttered with error messages!

I really want to figure out what's going on with Vuetify test (https://github.com/testing-library/vue-testing-library/pull/139/files#r420852127) before merging this!

spacedawwwg · 2020-05-20T07:35:56Z

@afenton90 …fancy seeing you here! 🤣

afontcu · 2020-05-20T16:32:20Z

🎉 This PR is included in version 5.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

mediafreakch · 2020-06-17T17:37:28Z

@afontcu Just to let you know: @vue/[email protected] now stubs transitions by default. After updating, some of my tests were failing because of this. The solution was to disable stubbing for transitions for those tests:

it('should mount', () => {
  /* MyComponent wrapped <v-snackbar /> which uses <transition> */
  const { getByText } = render(MyComponent, {
    props: {},
    stubs: {
      transition: false;
    }
  })
})

Maybe this belongs to the release docs here to avoid someone else having to google the solution :)

After updating I also have Typescript errors coming from @types/vue-testing-library__vue:

ERROR in .../node_modules/@types/testing-library__vue/node_modules/vuex/types/vue.d.ts(10,5):
10:5 Subsequent property declarations must have the same type.  Property 'store' must be of type 'Store<any> | undefined', but here has type 'Store<any> | undefined'.
     8 | declare module "vue/types/options" {
     9 |   interface ComponentOptions<V extends Vue> {
  > 10 |     store?: Store<any>;
       |     ^
    11 |   }
    12 | }
    13 |

I temporarily disabled the errors by adding the following to my tsconfig.json:

...,
compilerOptions: {
  skipLibCheck: true
}

afontcu · 2020-06-17T17:52:24Z

Hi! Thanks for the insights. How did your test break? Sync vs. async behaviour? Could you share the failing component / test?

Property 'store' must be of type 'Store<any> | undefined', but here has type 'Store<any> | undefined'.

you gotta love typescript errors 😅

mediafreakch · 2020-06-17T18:17:19Z

@afontcu Here's the failing test:

describe('ErrorMessage', () => {
  it('should not be visible by default', () => {
    const { queryByTestId } = render(ErrorMessage, {})

    expect(queryByTestId('snackbar')).toBe(null)
  })
})

The test failed with:

Expected: null
Received: <transition-stub data-testid="snackbar" name="v-snack-transition" />

Here the tested component (I removed some parts that are unrelated to the initial render):

<template>
  <v-snackbar :value="isVisible" multi-line :timeout="0" color="secondary" data-testid="snackbar">
    <span>
      {{ message }}
    </span>
  </v-snackbar>
</template>

<script lang="ts">
  import Vue from 'vue'

  export default Vue.extend({
    props: {
      errorCode: Number,
      errorIsIntrusive: {
        type: Boolean,
        default: false
      },
    },
    data() {
      return {
        isVisible: false
      }
    },
    watch: {
      errorCode: {
        handler: function(newVal) {
          if (newVal) {
            // we show only if error is not intrusive (intrusive errors are handled elsewhere)
            this.isVisible = !!newVal && !this.errorIsIntrusive
          }
        },
        immediate: true
      },
    },
    computed: {
      message(): string {
        return `${this.$t('Error.TextForCode', { code: this.errorCode })}`
      }
    },
  })
</script>

afontcu · 2020-06-18T06:31:41Z

Cool! Thanks for the info. I'll add the information in the v5.0.3 release, so that people can find it easily.

feat(@vue/test-utils): upgrade to v1

8231b0d

afontcu reviewed May 6, 2020

View reviewed changes

refactor(vtu): upgrade to 1.0.2

1582bc6

removes config for depracation notice as isVueInstance is now removed from internals

afontcu mentioned this pull request May 8, 2020

Using external libraries #140

Closed

Update build-related packages

0ac8f26

afontcu added 2 commits May 16, 2020 21:46

Remove removed sync option

8d76484

Bump DOM Testing Library

13ba03e

Bump to VTU 1.0.3

d47df70

afontcu added 4 commits May 20, 2020 18:12

Fix Vuetify test

902d899

Remove test unrelated to Vuetify

3831452

Bump DTL patch version

d6087aa

Add @afenton90 as a contributor

dc38df6

afontcu changed the title ~~feat(@vue/test-utils): upgrade to v1~~ fix: update VTU to v1, update types May 20, 2020

afontcu merged commit 7a7c0fd into testing-library:master May 20, 2020

afontcu added the released label May 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update VTU to v1, update types #139

fix: update VTU to v1, update types #139

afenton90 commented May 6, 2020 •

edited by afontcu

Loading

afenton90 commented May 6, 2020

afontcu May 6, 2020

afenton90 May 6, 2020 •

edited

Loading

afenton90 May 6, 2020

lmiller1990 May 16, 2020

lmiller1990 May 16, 2020

afontcu May 20, 2020

afenton90 May 20, 2020

afontcu May 20, 2020

afenton90 May 20, 2020

afontcu May 20, 2020

marcpeabody commented May 12, 2020

afontcu commented May 12, 2020 •

edited

Loading

afenton90 commented May 13, 2020

afontcu commented May 13, 2020

codecov bot commented May 13, 2020 •

edited

Loading

kuddl commented May 15, 2020

afontcu commented May 16, 2020

spacedawwwg commented May 20, 2020

afontcu commented May 20, 2020

mediafreakch commented Jun 17, 2020

afontcu commented Jun 17, 2020 •

edited

Loading

mediafreakch commented Jun 17, 2020

afontcu commented Jun 18, 2020

fix: update VTU to v1, update types #139

fix: update VTU to v1, update types #139

Conversation

afenton90 commented May 6, 2020 • edited by afontcu Loading

afenton90 commented May 6, 2020

Choose a reason for hiding this comment

afenton90 May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcpeabody commented May 12, 2020

afontcu commented May 12, 2020 • edited Loading

afenton90 commented May 13, 2020

afontcu commented May 13, 2020

codecov bot commented May 13, 2020 • edited Loading

Codecov Report

kuddl commented May 15, 2020

afontcu commented May 16, 2020

spacedawwwg commented May 20, 2020

afontcu commented May 20, 2020

mediafreakch commented Jun 17, 2020

afontcu commented Jun 17, 2020 • edited Loading

mediafreakch commented Jun 17, 2020

afontcu commented Jun 18, 2020

afenton90 commented May 6, 2020 •

edited by afontcu

Loading

afenton90 May 6, 2020 •

edited

Loading

afontcu commented May 12, 2020 •

edited

Loading

codecov bot commented May 13, 2020 •

edited

Loading

afontcu commented Jun 17, 2020 •

edited

Loading