[POOL-606] oc-chef-pedant: improve oc_id API test coverage #1289
Conversation
This does not involve controllers/routes that: a) send an email (we can't do that on prem) b) require an existing oauth2 application c) zendesk Profile update does not include the email address because this, too, would send an email. Signed-off-by: Stephan Renatus <[email protected]>
Signed-off-by: Stephan Renatus <[email protected]>
|
|
||
| # create user before each test so that we don't regen the keys | ||
| # of the standard user used in other tests. | ||
| let(:oc_id_user) { platform.create_user(username, user_overrides) } |
There was a problem hiding this comment.
nit: Could we do this as shared(:oc_id_user) and clean it up in an after(:all)? Disregard if the tests change user record state in a way that would affect other tests. It's not a huge deal, I'm just hoping to avoid increasing overall pedant time where it can be done safely.
There was a problem hiding this comment.
Cool. I didn't know how to do that. 👍
There was a problem hiding this comment.
I've taken a stab at doing this but it gets a bit ugly to be honest since: (1) As far as I can tell shared isn't a thing, so you need to use instance variables in a before block and (2) we have at least 3 tests that change the user-object. We can order those tests precisely so they don't conflict, but I think it is just best to eat the extra tests for now.
There was a problem hiding this comment.
That works for now. shared is a pedant thing that's hiding somewhere in our libs. Semantically similar to let but less robust for overriding iirc, and it persists across examples so that they can be accessed for setup in before/after :all blocks.
There was a problem hiding this comment.
Hrm, perhaps we've broken it then, because it super-duper didn't work right for me. BUT, it could be that I was actually hitting some of the data interactions.
| @@ -2,6 +2,38 @@ | |||
| require 'openssl' | |||
|
|
|||
There was a problem hiding this comment.
Would it make sense to include a comment specifying endpoints that exist but are not covered, with a reference to the unit coverage? The possible down side is remembering to update that when it changes - but Id endpoints are relatively static.
There was a problem hiding this comment.
👍 can do. It makes sense to not only have that information in the Jira ticket, but also where you might want to see it. 😉
There was a problem hiding this comment.
I added a new untested_endpoint method that we can use to document issues like this.
Signed-off-by: Steven Danna <[email protected]>
There was a problem hiding this comment.
Addressed some of the comments. @marcparadise Let me know what you think regarding the user creation. I have a single-user setup working locally, it is just a bit ugly.
| @@ -387,6 +387,16 @@ def self.isolate(message = nil, &examples) | |||
| end | |||
| end | |||
|
|
|||
| # Used to easily documented endpoints that don't have tests | |||
This does not involve controllers/routes that:
a) send an email (we can't do that on prem)
b) require an existing oauth2 application
c) zendesk
Profile update does not include the email address because this, too,
would send an email.