System Batch UI, Client Status Bar Chart and Client Tab page view #11078
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChaiWithJai
force-pushed
the
f-sysbatch-ui
branch
from
978f087 to
1f95a67
Compare
Ember Asset Size actionAs of 8c8c3fd Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
Ember Test Audit flaky testsEmber Test Audit detected these flaky tests on 38f764f:
|
Ember Test Audit flaky testsEmber Test Audit detected these flaky tests on 2c96d75:
|
refactor forceCollapsed logic to use hasClientStatus prop
Ember Test Audit flaky testsEmber Test Audit detected these flaky tests on c38c0a5:
|
Ember Test Audit flaky testsEmber Test Audit detected these flaky tests on dc22c75:
|
ChaiWithJai
commented
Oct 7, 2021
🎉
lgfa29
added a commit
that referenced
this pull request
Oct 8, 2021
lgfa29
added a commit
that referenced
this pull request
Oct 14, 2021
| @@ -25,7 +25,8 @@ module.exports = function(environment) { | |||
|
|
|||
| APP: { | |||
| blockingQueries: true, | |||
| mirageScenario: 'topoMedium', | |||
| // TODO: revert before merging to main. | |||
| mirageScenario: 'sysbatchSmall', // convert to 'sysbatchSmall' when working on feature | |||
There was a problem hiding this comment.
Oops. topoMedium isn't a particularly good default scenario anyway.
Comment on lines
+63
to
+65
| const clientIDs = clients.sortBy('id').map(c => c.id); | ||
| const clientsInTable = Clients.clients.map(c => c.id).sort(); | ||
| assert.deepEqual(clientsInTable, clientIDs); |
There was a problem hiding this comment.
Ideally page object collections aren't sorted, and instead of the fixture data is sorted to match what the app behavior is. Either way, good to have coverage here.
Comment on lines
+84
to
+85
| assert.true(hasTooltip, `row ${index} has ${col} tooltip`); | ||
| assert.ok(tooltipText, `row ${index} has ${col} tooltip content ${tooltipText}`); |
There was a problem hiding this comment.
These two assertion methods do the same thing. assert.ok is the method we have used in Nomad.
Comment on lines
+19
to
+26
| moduleForJobWithClientStatus('Acceptance | job detail with client status (system)', () => | ||
| server.create('job', { | ||
| status: 'running', | ||
| datacenters: ['dc1'], | ||
| type: 'system', | ||
| createAllocations: false, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
No harm in continuing the pattern of creating new moduleFor* utilities, but now the more idiomatic thing to do is use hooks to add common behavior. Something like:
module('Acceptance | job detail with client status (system)', function(hooks) {
setupJobWithClientStatus(hooks);
test('...');
});
Comment on lines
+195
to
+199
| for (var testName in additionalTests) { | ||
| test(testName, async function(assert) { | ||
| await additionalTests[testName].call(this, job, assert); | ||
| }); | ||
| } |
There was a problem hiding this comment.
For annoying legacy JavaScript reasons, this would be better as a for...of loop.
|
|
||
| assert.deepEqual(result, expected); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Great test coverage on the hairy bits of this util 👏
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is linked to this umbrella issue.
It should cover the following:
sysbatchjobsperiodicanddispatchsysbatchjobsClientstabCases to test:
periodicandparameterizedsysbatchjobssysbatchjob is registered