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

Creates tests for clasp clone and clasp pull #119

Merged
merged 4 commits into from Apr 20, 2018
Merged

Creates tests for clasp clone and clasp pull #119

merged 4 commits into from Apr 20, 2018

Conversation

campionfellin
Copy link
Collaborator

When running clasp clone test make sure you put in a script ID.

Signed-off-by: campionfellin campionfellin@gmail.com

When running clasp clone test make sure you put in a script ID.

Signed-off-by: campionfellin <campionfellin@gmail.com>
@coveralls
Copy link

coveralls commented Apr 18, 2018

Pull Request Test Coverage Report for Build 74

  • 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
  • 149 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-60.8%) to 10.0%

Files with Coverage Reduction New Missed Lines %
tests/test.js 12 57.89%
index.js 137 7.94%
Totals Coverage Status
Change from base Build 63: -60.8%
Covered Lines: 85
Relevant Lines: 631

💛 - Coveralls

Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

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

Looks pretty good.
2 comments.

tests/test.ts Outdated
@@ -43,6 +43,31 @@ describe.skip('Test clasp create function', () => {
});
});

// When running this test, please put in a script ID
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean "put in a script ID"?
Can we first create a test script ID, then clone it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, we can do one and hard code it for everyone, if you think that's better. Or use the one from the clasp create test.

tests/test.ts Outdated
@@ -43,6 +43,31 @@ describe.skip('Test clasp create function', () => {
});
});

// When running this test, please put in a script ID
// then use `sudo npm run build` before running test.
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need to build before any test?

Shouldn't we have this in the file header comment if that is the case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah good point

@grant
Copy link
Contributor

grant commented Apr 18, 2018

Not sure where the -61.02% came from.

@campionfellin
Copy link
Collaborator Author

campionfellin commented Apr 18, 2018

Not sure on the coveralls issue either...

Looks like it's been pretty steady at 9.32% and then jumped up to 70.83% on your build... Now it is back down to 9.82% which is a slight increase.

Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

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

Marking "Request changes" in GH for the comments.

It now uses the .clasp.json file made by the clasp create function before it

Signed-off-by: campionfellin <campionfellin@gmail.com>
Signed-off-by: campionfellin <campionfellin@gmail.com>
Signed-off-by: campionfellin <campionfellin@gmail.com>
@campionfellin
Copy link
Collaborator Author

Though coveralls will likely report differently, it looks like this brings overall coverage to 36.89%

@grant grant merged commit 72b0379 into google:master Apr 20, 2018
@grant
Copy link
Contributor

grant commented Apr 20, 2018

Seeing 7.94 /index.js. It's a bit weird since the ts is compiled then covered.
I think coverage for tests/test.js is high, not sure what that means though.

@campionfellin
Copy link
Collaborator Author

So this is because some of the tests can't be run on Travis so they are skipped. If you run locally and un-skip every test, this is the output:

-------------|----------|----------|----------|----------|-------------------|
File         |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-------------|----------|----------|----------|----------|-------------------|
All files    |    36.14 |    24.05 |    33.66 |    36.89 |                   |
 clasp       |    32.48 |    24.05 |     29.9 |    32.89 |                   |
  index.js   |    32.48 |    24.05 |     29.9 |    32.89 |... 1285,1294,1298 |
 clasp/tests |      100 |      100 |      100 |      100 |                   |
  test.js    |      100 |      100 |      100 |      100 |                   |
-------------|----------|----------|----------|----------|-------------------|

@campionfellin campionfellin deleted the more-tests branch April 22, 2018 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants