-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
UI/vault 9268/pki component tests #17609
Conversation
@@ -19,7 +19,7 @@ export default Helper.extend({ | |||
const currentRoute = router.get('currentRouteName'); | |||
let currentURL = router.get('currentURL'); | |||
// if we have any query params we want to discard them | |||
currentURL = currentURL.split('?')[0]; | |||
currentURL = currentURL?.split('?')[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this errors out in the tests. It happens because this is the first time SecretListHeader
is being using in an engine, and an engine has different URLs and routes passed into this helper then it does outside an engine.
id="not_after" | ||
autocomplete="off" | ||
spellcheck="false" | ||
value={{this.notAfter}} | ||
{{on "input" this.setAndBroadcastInput}} | ||
class="input" | ||
maxLength="21" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my original thought here was that a utc timestamp wouldn't go past this length, but that's not true. To stay safe, I just removed the restriction.
@@ -19,7 +19,7 @@ export default Helper.extend({ | |||
const currentRoute = router.get('currentRouteName'); | |||
let currentURL = router.get('currentURL'); | |||
// if we have any query params we want to discard them | |||
currentURL = currentURL.split('?')[0]; | |||
currentURL = currentURL?.split('?')[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This erroring out as the currentURL returns a little differently in an engine vs. non-engine.
@@ -49,7 +49,7 @@ | |||
data-test-input={{attr.name}} | |||
> | |||
{{#each (path-or-array attr.options.possibleValues @model) as |val|}} | |||
<option selected={{eq (get @model this.valuePath) (or val.value val)}} value={{or val.value val}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
caused an issue with the "Default time of 0" selection for signature_bits
assert.strictEqual( | ||
this.model.signatureBits, | ||
0, | ||
'sets the default value for signature_bits on the model.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
circling back to this flow because the default value comes from the defined default value on the model, but if you reselect this drop down, then it comes from the possibleValues on the model. I noticed a bug here (fixed) but figured it was a good idea to test for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome amount of tests thanks! I left a couple of comments but nothing blocking.
|
||
module('Integration | Component | pki-key-parameters', function (hooks) { | ||
setupRenderingTest(hooks); | ||
setupEngine(hooks, 'pki'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
assert.deepEqual(this.model.keyType, 'rsa', 'sets the default value for key_type on the model.'); | ||
assert.deepEqual(this.model.keyBits, 2048, 'sets the default value for key_bits on the model.'); | ||
assert.deepEqual(this.model.signatureBits, 0, 'sets the default value for signature_bits on the model.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you want strictEqual
here. deepEqual
is to compare objects. You could also use propEqual
to bundle into one assertion:
assert.propEqual(this.model,
{
keyType: 'rsa',
keyBits: 2048,
signatureBits: 0,
},
'sets default model values'
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And very cool about propEqual!
assert.deepEqual( | ||
this.model.extKeyUsage, | ||
undefined, | ||
'sets no default value set for ext_key_usage on load.' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use strictEqual
here instead
* wip * work in progress * pki-role-form-test * clean up * radio-select-ttl-or-string test * clean up * add yielded check * 12 to 13 * add pki-key-usage test * remove meep * key-params test * clean up * clean up * pr comments
This PR covers the following tests. These were all new components used in the PKI Role Create view.
Tests:
PkiRoleForm
RadioSelectTtlOrString
PkiKeyUsage
PkiKeyParameters