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

[Cookbook] Unit Testing components #1369

Merged
merged 74 commits into from
Jan 31, 2018
Merged

[Cookbook] Unit Testing components #1369

merged 74 commits into from
Jan 31, 2018

Conversation

lmiller1990
Copy link
Member

For #1344

First draft. Some high level feedback would be great.

  • am I heading the right direction?
  • are the examples too simple/complex?
  • general structure/content

@chrisvfritz chrisvfritz requested a review from sdras January 5, 2018 19:01
Jinjiang and others added 3 commits January 5, 2018 14:06
* added english srt file

* added chinese srt file

* typo
Change function execution order to correct sidebar anchor targets; more concise fix to #1348.
Hi,
The images link for `hn.png` and `hn-architecture.png` can be found on `../../images/`.
@sdras
Copy link
Member

sdras commented Jan 5, 2018

Excellent! I'm slammed at work today but can't wait to look it over this weekend. Thanks!

MachinisteWeb and others added 13 commits January 5, 2018 14:12
* New in with + symbol

Signed-off-by: Bruno Lesieur <[email protected]>

* Review of 2.5.0 doc

Signed-off-by: Bruno Lesieur <[email protected]>

* Review

Signed-off-by: Bruno Lesieur <[email protected]>

* Fix syntax typo

Signed-off-by: Bruno Lesieur <[email protected]>

* Add space between new line of documentation

Signed-off-by: MachinisteWeb <[email protected]>

* Add @posva review

Signed-off-by: MachinisteWeb <[email protected]>
The tweeningValue no longer seems to be available in the tween object itself. Instead, the tweeningValue is available in the tweened object that passed as a parameter to the onUpdate callback.
* Add watch usages!

Add `watch` usages!

* Update index.md

* tweaks to watch api examples
* Added video into guide introduction

* Added modal styles to page, put video line on one file, and reset iframe margin

## Simple Example

Unit testing is a fundamental part of software developmenet. The underlying idea is tests the smallest units of code, in isolation from others. This makes it easy to refactor, add new features, or track down bugs. Vue's [single-file components](./single-file-components.html), it is straight forward to write unit tests for components in isolation. This lets you develop new features with confidence you are not breaking existing ones, and helps other people to understand what your component does.
Copy link
Member

Choose a reason for hiding this comment

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

Great opening, a couple of things:

  1. s/developmenet/development
  2. Second sentence isn't grammatically correct, how about something like: Unit tests evaluate the smallest units of code in isolation, in order to track down bugs, or make it easier to track down bugs or refactor.
  3. No comma necessary after link to sfc- so it would be "Vue's single file components make it straightforward..."

})
```

This simple example shown how to test whether an error message is rendered based on the length of the username. It demonstrates the general idea of unit testing Vue components: render the component, and make assertions to check if the markup matches the state of the component.
Copy link
Member

Choose a reason for hiding this comment

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

You said simple example earlier, how about "This shows how to ..."

[vue-test-utils](https://github.com/vuejs/vue-test-utils) is the official library for unit testing Vue components. The (vue-cli)[https://github.com/vuejs/vue-cli] webpack template comes with either Karma or Jest, both well supported test runners, and there are some (guides)[https://vue-test-utils.vuejs.org/en/guides/] in the `vue-test-utils` documentation.


4. Discuss why this may be a compelling pattern. Links for reference are not required but encouraged.
Copy link
Member

Choose a reason for hiding this comment

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

This is from the template, should be removed :)


This simple example shown how to test whether an error message is rendered based on the length of the username. It demonstrates the general idea of unit testing Vue components: render the component, and make assertions to check if the markup matches the state of the component.

## Details about the Value
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need to literally say "details about the value, I think you could change the h2 directly to "Why Test"

Unit tests should be
- Fast to run
- Easy to understand
- Only test a _single unit of work_
Copy link
Member

Choose a reason for hiding this comment

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

Perfect. Straightforward, easy to reproduce, wonderful list and explanation.

The below example improves the test by:
- only making one assertion per `it` block
- having short, clear test descriptions
- providing only the minimum data requires for the test
Copy link
Member

Choose a reason for hiding this comment

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

Great, again, good job being clear and concise. The demo illustrates exactly this as well.

@sdras
Copy link
Member

sdras commented Jan 28, 2018

Hi @lmiller1990!
I think I know why this is failing- you designated the order of this cookbook entry as the same as one that already exists- form validation. Can you please update to 1.3 or 1.4? Thanks!

const wrapper = shallow(Hello)

// should not allow for username less than 7 characters, excludes whitespace
wrapper.setData({ username: ' '.repeat(7) })

Choose a reason for hiding this comment

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

Nice addition. <3

Copy link

@codebryo codebryo left a comment

Choose a reason for hiding this comment

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

I would approve this document. I found one extra word that slipped in but an otherwise great summary. I like the state of this a lot. Great work.


Points to note:

At the top, we declare the factory function, which simply returns a new `wrapper` instance, and sets and the `values` object to `data`. This way, we don't need to duplicate `const wrapper = shallow(Foo)` in every test. Another great benefit to this is when more complex components with a method or computed property you might want to mock or stub in every test, you only need to declare it once.

Choose a reason for hiding this comment

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

and sets and the values

Choose a reason for hiding this comment

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

That second part of the sentence in general seems a little off.
What about:

At the top, we declare the factory function, which simply returns a new wrapper instance, and merges thevalues object into data.

@lmiller1990
Copy link
Member Author

Thanks both, I'll update with @codebryo 's points and change entry number tonight!

@lmiller1990
Copy link
Member Author

Fixed based on comments. The build is STILL failing despite changing the order.

The error is 2:01:22 AM: npm ERR! missing script: build. There is indeed no build in package.json, but I don't feel like this is the real problem. Anyone know?

@sdras
Copy link
Member

sdras commented Jan 30, 2018

It might be due to some changes that were made between the time you forked and when you submitted the PR. The weird part is that you have only changed that one file. Can you please try to rebase? If that doesn't work, I'll look into it more and get it merged for you.

@lmiller1990
Copy link
Member Author

Rebasing did the trick!

@sdras sdras merged commit 813ab53 into vuejs:master Jan 31, 2018
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.