UI: Add removal of OTT query parameter with delay #10319
Conversation
Ember Asset Size actionAs of 7153b67 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
I’m not a fan of having to do it this way but everything else I tried failed.
backspace
force-pushed
the
b-ui/ott-clear
branch
from
6428340 to
8b656d8
Compare
| @@ -169,6 +169,9 @@ module('Acceptance | tokens', function(hooks) { | |||
| const { oneTimeSecret, secretId } = managementToken; | |||
|
|
|||
| await JobDetail.visit({ id: job.id, ott: oneTimeSecret }); | |||
|
|
|||
| assert.notOk(currentURL().includes(oneTimeSecret), 'OTT is cleared from the URL after loading'); | |||
There was a problem hiding this comment.
Does this working imply that all our job detail tests are now 500ms longer?
There was a problem hiding this comment.
whew, good question… maybe it would make ALL acceptance tests longer since it’s in the application route! I’ll look more into this, thanks.
There was a problem hiding this comment.
The test audit action suggests it didn't. 🤷
There was a problem hiding this comment.
I don’t know… before I changed the default value from null to '', this was the first audit:
The utility of it as it exists is in doubt to me, although I guess it would at least catch if something was DRAMATICALLY slower.
To hopefully reduce the variability from what happens on in CircleCI, I ran locally with 5 repetitions and it was two minutes slower with the global delay vs the scoped delay I added in 7153b67. (9m 23s 746ms vs 7m 23s 150ms) The unsightliness is increased within that commit, as it involves making the existence of the OTT query parameter checkable in setupController but it seems worth it to me to not drag on other tests. Thoughts? 😯
There was a problem hiding this comment.
Definitely worth it!
This is more unsightliness, but it seems to speed things up, at least locally… 🤔
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This is a follow-on to #10066 to clear out the OTT from the URL after loading. While this approach
is unsightly, it’s how I was able to get it to happen, and it seems less unsightly than having the OTT
persist in the URL.
Here’s a GIF showing it being cleared when passing a job to
nomad ui -authenticate: