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

Added unit and e2e tests for HelpSection.vue component #144

Merged
merged 8 commits into from
Apr 3, 2020

Conversation

hemanth-hk
Copy link
Contributor

Fixes #116

Description

I have added the unit and e2e tests for the HelpSection.vue component. And tested it locally

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the master branch of the repository.
  • My commit messages follow best practices.
  • I added tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no
    visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open-source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open-source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@hemanth-hk hemanth-hk requested review from a team March 23, 2020 18:42
@hemanth-hk
Copy link
Contributor Author

I was able to mock the $ga of Vue analytics but the method using it, was not returning anything, so I had to return something, hence I returned the return of the $ga.event function

Please review and check if I have left any cases like last time

@akmadian
Copy link
Member

@hemanth-hk We don't need to mock ga as there's no way I know of to verify that the event was received by ga without writing a bunch more code. Please remove all ga stuff and I'll have another look :)

@hemanth-hk
Copy link
Contributor Author

When I removed the ga stuff the test coverage was not 100%

@hemanth-hk
Copy link
Contributor Author

This is the screenshot of the unit test before mocking $ga

before-ga-stuff


This is the screenshot of the unit test after mocking $ga

after-ga-stuff

hemanth-hk and others added 3 commits March 31, 2020 15:50
Signed-off-by: Olga Bulat <obulat@gmail.com>
Signed-off-by: Olga Bulat <obulat@gmail.com>
@obulat
Copy link
Contributor

obulat commented Apr 3, 2020

When I removed the ga stuff the test coverage was not 100%

It's ok for coverage not to be 100% in this case.

Copy link
Contributor

@obulat obulat left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, @hemanth-hk !
I removed GA mocking, and will merge this PR.

@obulat obulat merged commit 64c2d98 into creativecommons:master Apr 3, 2020
@hemanth-hk
Copy link
Contributor Author

@obulat I would have removed them if you have just left a comment but please make sure you also remove the return statement form the Helpsection.vue component which I added to check it

@obulat
Copy link
Contributor

obulat commented Apr 3, 2020

Sorry about that, @hemanth-hk . I wanted to add some changes to quickly clean up errors in the repo to make it easier for other contributors to work on it, that is why I added some more changes here. Unfortunately, I didn't notice the return statement. Could you please create a PR removing it?

@hemanth-hk
Copy link
Contributor Author

I added them in #154, That would be fine right?

@obulat
Copy link
Contributor

obulat commented Apr 3, 2020

Sure, you acted before I could reply. Thank you again, the repo is linted and test run well now!

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.

Add unit and e2e tests for the HelpSection component
3 participants