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

Add more tests #182

Merged
merged 3 commits into from Jun 28, 2018
Merged

Add more tests #182

merged 3 commits into from Jun 28, 2018

Conversation

ofrobots
Copy link
Contributor

In an effort to improve coverage, I started working some tests. It turns out that our current coverage numbers are a lie as init.ts wasn't accounted for. It only gets tested via test-kitchen.ts which is hard to report for coverage purposes (it uses an tarball-installed version of the package rather than the existing files).

Here some more tests. Still more to come, but no reason to sit on these. These can land.

@codecov-io
Copy link

Codecov Report

Merging #182 into master will decrease coverage by 0.97%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #182      +/-   ##
==========================================
- Coverage   99.13%   98.15%   -0.98%     
==========================================
  Files          10       12       +2     
  Lines         460      652     +192     
  Branches       31       55      +24     
==========================================
+ Hits          456      640     +184     
- Misses          4       12       +8
Impacted Files Coverage Δ
test/test-init.ts 100% <100%> (ø)
src/init.ts 88.99% <100%> (ø)
test/test-format.ts 100% <100%> (ø) ⬆️
src/format.ts 100% <100%> (+6.81%) ⬆️
src/util.ts 100% <0%> (+5.88%) ⬆️

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 122fe81...08989fb. Read the comment docs.

files: string[];
license: string;
keywords: string[];
name?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be that I'm missing context - why are these all optional now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They should always have been optional. The schema for package.json isn't very strong and there is no requirement that all of these keys must be present.

@@ -37,7 +37,7 @@ const OPTIONS: Options = {
logger: {log: console.log, error: console.error, dir: nop}
};

test.serial('format should return false for well-formatted files', t => {
test.serial('format should return true for well-formatted files', t => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well that's confusing :) Was the title of the test just wrong before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true

@ofrobots ofrobots merged commit b2b5bbd into google:master Jun 28, 2018
@ofrobots ofrobots deleted the test-init branch June 28, 2018 06:13
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