#66 test: 100% coverage #78

acdpnk · 2016-02-25T15:03:36Z

WIP

gr2m · 2016-02-25T16:22:50Z

test/unit/fetch-test.js

+    bearerToken: 'abc45678',
+    path: false
+  }, 'calls fetchProperties with account url and non-string path')
+  // expected behavior??


path in this context is not part of the URL, but the path to the properties you want to return. If it’s undefined, all properties are returned. If path is set to address, then only the address value is returned. false makes not much sense here, why did you decide to test with it?

See examples in the README for account.profile.fetch

Note that this example is wrong – I’ll make a separate issue to get it fixed

account.fetch('fullname').then(function (createdAt) { alert('Hey there ' + fullname) })

should be

account.fetch('fullname').then(function (fullname) { alert('Hey there ' + fullname) })

false makes not much sense here, why did you decide to test with it?

To achieve full branch coverage I need to have a test covering the else clause in the following code.

if (typeof path === 'string') { set(state.account, path, properties) } else { merge(state.account, properties) }

Which, if I'm not mistaken, comes down to testing with a path where typeof path !== 'string'. I picked false because it doesn't make much sense, as I assumed that this was not a legitimate use case.

ah, I think you can leave out path entirely, so that it will be undefined, that will cover the else path

gr2m · 2016-03-04T19:06:18Z

Hey @chrifpa, what’s your status here?

acdpnk · 2016-03-05T16:21:13Z

Got sick and had to stop working on it for a bit. Back now.

gr2m · 2016-03-05T17:44:20Z

Glad to hear you are good again, thanks for the update! Ping us if you have any questions :)

gr2m · 2016-03-18T22:02:33Z

Hey @chrifpa all good from your side? Any blockers right now?

acdpnk · 2016-03-20T14:39:38Z

Just a little constrained on time, sorry… 😄

acdpnk · 2016-03-22T17:57:15Z

test/unit/utils-deserialise-test.js

+  t.deepEqual(data, accountsReturn, 'returns right data')
+
+  t.end()
+})


Hey @gr2m, do you mind having a look at this? I wrote these tests to achieve full branch coverage, but I have a hard time figuring out if the fixtures I'm using make sense and/or if this last test — with what to me looks like a malformed response — shouldn't fail.

sure looking into it now, thanks @chrifpa

for the complex relationship, instead of having account -> profile -> vet, can you do session -> account -> profile? You can take this as a server response fixture: https://github.com/hoodiehq/hoodie-server-account/blob/master/tests/integration/fixtures/session-with-profile-response.json

The rest looks great, thanks for the enormous efforts Chris! 💐

Thanks, will do!

gr2m · 2016-03-22T18:01:29Z

sorry for the noisy comments by coveralls, I’ve disabled that now

gr2m · 2016-03-22T21:44:55Z

Looking great now, also getting close :)

=============================== Coverage summary ===============================
Statements   : 99.39% ( 492/495 )
Branches     : 99.28% ( 138/139 )
Functions    : 96.59% ( 85/88 )
Lines        : 99.39% ( 492/495 )
================================================================================

Let us know if you run into any blockers or have any questions, we know that some of these might be tricky to get 100% :)

acdpnk · 2016-03-24T16:13:22Z

test/fixtures/admin-signin.json

+      }
+    }
+  ]
+}


Hey @gr2m, could you take a look at this and tell me wether this fixture makes sense?

Admins have no account relationship, you can remove the "relationships" key and the "included" key

acdpnk · 2016-03-24T18:21:53Z

Looks like coveralls got stuck. Anything I can do to unstick it?

finnp · 2016-03-24T20:33:43Z

@chrifpa I re-run the travis job and now it's working. 100% ftw \o/

gr2m · 2016-03-24T20:35:06Z

w00p now that looks great 👏

=============================== Coverage summary ===============================
Statements   : 100% ( 495/495 )
Branches     : 100% ( 137/137 )
Functions    : 100% ( 87/87 )
Lines        : 100% ( 495/495 )
================================================================================

gr2m · 2016-03-24T20:37:02Z

test/integration/admin-signin-test.js

+
+    var storeAccount = store.getObject('account_admin')
+
+    t.is(storeAccount, null, 'removes acocunt from store')


typo: acocunt should be account

gr2m · 2016-03-24T20:40:16Z

okay if you could fix the fixture and the typo, and clean up the commits, this is good to be merged :)

acdpnk · 2016-03-25T11:26:51Z

I think I got the commits right now 😅

Therefore ready for review @gr2m

gr2m · 2016-03-25T22:21:51Z

Yes, absolutely, perfect, wow such work, very awesome 🐶 👏

LGTM!

ping @hoodiehq/maintainers for 2nd 👀 review

gr2m · 2016-03-25T22:22:53Z

@chrifpa can you rebase your work on latest master? No biggie if not, the 2nd reviewer can do the rebase before merging, and then manually close this PR and the issue

acdpnk · 2016-03-26T06:58:47Z

Thanks @gr2m :)

Rebase on latest master

christophwitzko · 2016-03-26T10:28:45Z

LGTM 👍

gr2m reviewed Feb 25, 2016
View reviewed changes

gr2m mentioned this pull request Feb 25, 2016

README: example for account.profile.fetch() has a bug #79

Closed

toh82 mentioned this pull request Mar 8, 2016

Bring code coverage to 100% #66

Closed

6 tasks

acdpnk reviewed Mar 22, 2016
View reviewed changes

acdpnk reviewed Mar 24, 2016
View reviewed changes

gr2m reviewed Mar 24, 2016
View reviewed changes

acdpnk force-pushed the test-coverage branch 3 times, most recently from 1be6d29 to 86a9ce9 Compare March 25, 2016 11:21

acdpnk force-pushed the test-coverage branch from 86a9ce9 to ff7ea9c Compare March 25, 2016 18:19

acdpnk changed the title ~~#66 coverage~~ #66 test: 100% coverage Mar 25, 2016

acdpnk added 2 commits March 26, 2016 07:48

test: 100% coverage

b2fbd89

chore(admin): remove unused code

27a8133

acdpnk force-pushed the test-coverage branch from ff7ea9c to 27a8133 Compare March 26, 2016 06:48

christophwitzko merged commit 40a5186 into hoodiehq:master Mar 26, 2016

acdpnk deleted the test-coverage branch March 26, 2016 12:52

gr2m mentioned this pull request Apr 6, 2016

Bring code coverage to 100% hoodiehq/pouchdb-admins#11

Open

gr2m mentioned this pull request Apr 19, 2016

shout out blog post hoodiehq/editorial#88

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#66 test: 100% coverage #78

#66 test: 100% coverage #78

acdpnk commented Feb 25, 2016

gr2m Feb 25, 2016

acdpnk Feb 26, 2016

gr2m Feb 26, 2016

gr2m commented Mar 4, 2016

acdpnk commented Mar 5, 2016

gr2m commented Mar 5, 2016

gr2m commented Mar 18, 2016

acdpnk commented Mar 20, 2016

acdpnk Mar 22, 2016

gr2m Mar 22, 2016

gr2m Mar 22, 2016

acdpnk Mar 22, 2016

gr2m commented Mar 22, 2016

gr2m commented Mar 22, 2016

acdpnk Mar 24, 2016

gr2m Mar 24, 2016

acdpnk commented Mar 24, 2016

finnp commented Mar 24, 2016

gr2m commented Mar 24, 2016

gr2m Mar 24, 2016

gr2m commented Mar 24, 2016

acdpnk commented Mar 25, 2016

gr2m commented Mar 25, 2016

gr2m commented Mar 25, 2016

acdpnk commented Mar 26, 2016

christophwitzko commented Mar 26, 2016


		var storeAccount = store.getObject('account_admin')

		t.is(storeAccount, null, 'removes acocunt from store')

#66 test: 100% coverage #78

#66 test: 100% coverage #78

Conversation

acdpnk commented Feb 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gr2m commented Mar 4, 2016

acdpnk commented Mar 5, 2016

gr2m commented Mar 5, 2016

gr2m commented Mar 18, 2016

acdpnk commented Mar 20, 2016

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

gr2m commented Mar 22, 2016

gr2m commented Mar 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acdpnk commented Mar 24, 2016

finnp commented Mar 24, 2016

gr2m commented Mar 24, 2016

Choose a reason for hiding this comment

gr2m commented Mar 24, 2016

acdpnk commented Mar 25, 2016

gr2m commented Mar 25, 2016

gr2m commented Mar 25, 2016

acdpnk commented Mar 26, 2016

christophwitzko commented Mar 26, 2016