From 9ef79cd92c758029bc5018320769ceb7a199efed Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 7 Nov 2018 17:07:40 -0800 Subject: [PATCH 1/2] Correctly mark parameterized children as parameterized: true --- ui/mirage/factories/job.js | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/mirage/factories/job.js b/ui/mirage/factories/job.js index 096c66228847..970fc531d3fd 100644 --- a/ui/mirage/factories/job.js +++ b/ui/mirage/factories/job.js @@ -67,6 +67,7 @@ export default Factory.extend({ // It is the Parameterized job's responsibility to create // parameterizedChild jobs and provide a parent job. type: 'batch', + parameterized: true, payload: window.btoa(faker.lorem.sentence()), }), From 5801b79b8101d668e697a3abfc60b32f70fff355 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 7 Nov 2018 21:04:27 -0800 Subject: [PATCH 2/2] Show the correct template for parameterized job children --- ui/app/models/job.js | 15 ++++--- .../components/job-page/parts/summary.hbs | 2 +- ui/mirage/factories/job.js | 5 ++- ui/tests/acceptance/job-detail-test.js | 39 ++++++++++++------- ui/tests/helpers/module-for-acceptance.js | 12 ++++++ ui/tests/helpers/module-for-job.js | 26 ++++++++++++- ui/tests/pages/jobs/detail.js | 3 ++ 7 files changed, 78 insertions(+), 24 deletions(-) diff --git a/ui/app/models/job.js b/ui/app/models/job.js index 6db6ef676779..7ddfdb0d796d 100644 --- a/ui/app/models/job.js +++ b/ui/app/models/job.js @@ -26,11 +26,14 @@ export default Model.extend({ // Instances of periodic or parameterized jobs are false for both properties periodic: attr('boolean'), parameterized: attr('boolean'), + dispatched: attr('boolean'), periodicDetails: attr(), parameterizedDetails: attr(), - hasChildren: or('periodic', 'parameterized'), + hasChildren: computed('periodic', 'parameterized', 'dispatched', function() { + return this.get('periodic') || (this.get('parameterized') && !this.get('dispatched')); + }), parent: belongsTo('job', { inverse: 'children' }), children: hasMany('job', { inverse: 'parent' }), @@ -63,14 +66,14 @@ export default Model.extend({ function() { const type = this.get('type'); - if (this.get('periodic')) { - return 'periodic'; - } else if (this.get('parameterized')) { - return 'parameterized'; - } else if (this.get('parent.periodic')) { + if (this.get('parent.periodic')) { return 'periodic-child'; } else if (this.get('parent.parameterized')) { return 'parameterized-child'; + } else if (this.get('periodic')) { + return 'periodic'; + } else if (this.get('parameterized')) { + return 'parameterized'; } else if (JOB_TYPES.includes(type)) { // Guard against the API introducing a new type before the UI // is prepared to handle it. diff --git a/ui/app/templates/components/job-page/parts/summary.hbs b/ui/app/templates/components/job-page/parts/summary.hbs index 0bacf5c52de8..8d9d36d537cb 100644 --- a/ui/app/templates/components/job-page/parts/summary.hbs +++ b/ui/app/templates/components/job-page/parts/summary.hbs @@ -1,4 +1,4 @@ -{{#list-accordion source=(array job) key="id" startExpanded=isExpanded onToggle=(action persist) as |a|}} +{{#list-accordion data-test-job-summary source=(array job) key="id" startExpanded=isExpanded onToggle=(action persist) as |a|}} {{#a.head buttonLabel=(if a.isOpen "collapse" "expand")}}
diff --git a/ui/mirage/factories/job.js b/ui/mirage/factories/job.js index 970fc531d3fd..5afae688be4d 100644 --- a/ui/mirage/factories/job.js +++ b/ui/mirage/factories/job.js @@ -68,6 +68,7 @@ export default Factory.extend({ // parameterizedChild jobs and provide a parent job. type: 'batch', parameterized: true, + dispatched: true, payload: window.btoa(faker.lorem.sentence()), }), @@ -121,7 +122,7 @@ export default Factory.extend({ task_group_ids: groups.mapBy('id'), }); - const hasChildren = job.periodic || job.parameterized; + const hasChildren = job.periodic || (job.parameterized && !job.parentId); const jobSummary = server.create('job-summary', hasChildren ? 'withChildren' : 'withSummary', { groupNames: groups.mapBy('name'), job, @@ -187,7 +188,7 @@ export default Factory.extend({ }); } - if (job.parameterized) { + if (job.parameterized && !job.parentId) { // Create parameterizedChild jobs server.createList('job', job.childrenCount, 'parameterizedChild', { parentId: job.id, diff --git a/ui/tests/acceptance/job-detail-test.js b/ui/tests/acceptance/job-detail-test.js index 0ea4acfa0ea8..6b79f1bf669c 100644 --- a/ui/tests/acceptance/job-detail-test.js +++ b/ui/tests/acceptance/job-detail-test.js @@ -5,32 +5,43 @@ import moduleForJob from 'nomad-ui/tests/helpers/module-for-job'; import JobDetail from 'nomad-ui/tests/pages/jobs/detail'; import JobsList from 'nomad-ui/tests/pages/jobs/list'; -moduleForJob('Acceptance | job detail (batch)', () => server.create('job', { type: 'batch' })); -moduleForJob('Acceptance | job detail (system)', () => server.create('job', { type: 'system' })); -moduleForJob('Acceptance | job detail (periodic)', () => server.create('job', 'periodic')); +moduleForJob('Acceptance | job detail (batch)', 'allocations', () => + server.create('job', { type: 'batch' }) +); +moduleForJob('Acceptance | job detail (system)', 'allocations', () => + server.create('job', { type: 'system' }) +); +moduleForJob('Acceptance | job detail (periodic)', 'children', () => + server.create('job', 'periodic') +); -moduleForJob('Acceptance | job detail (parameterized)', () => +moduleForJob('Acceptance | job detail (parameterized)', 'children', () => server.create('job', 'parameterized') ); -moduleForJob('Acceptance | job detail (periodic child)', () => { +moduleForJob('Acceptance | job detail (periodic child)', 'allocations', () => { const parent = server.create('job', 'periodic'); return server.db.jobs.where({ parentId: parent.id })[0]; }); -moduleForJob('Acceptance | job detail (parameterized child)', () => { +moduleForJob('Acceptance | job detail (parameterized child)', 'allocations', () => { const parent = server.create('job', 'parameterized'); return server.db.jobs.where({ parentId: parent.id })[0]; }); -moduleForJob('Acceptance | job detail (service)', () => server.create('job', { type: 'service' }), { - 'the subnav links to deployment': (job, assert) => { - JobDetail.tabFor('deployments').visit(); - andThen(() => { - assert.equal(currentURL(), `/jobs/${job.id}/deployments`); - }); - }, -}); +moduleForJob( + 'Acceptance | job detail (service)', + 'allocations', + () => server.create('job', { type: 'service' }), + { + 'the subnav links to deployment': (job, assert) => { + JobDetail.tabFor('deployments').visit(); + andThen(() => { + assert.equal(currentURL(), `/jobs/${job.id}/deployments`); + }); + }, + } +); let job; diff --git a/ui/tests/helpers/module-for-acceptance.js b/ui/tests/helpers/module-for-acceptance.js index 328e5de58dcc..e18770ee4d67 100644 --- a/ui/tests/helpers/module-for-acceptance.js +++ b/ui/tests/helpers/module-for-acceptance.js @@ -20,5 +20,17 @@ export default function(name, options = {}) { let afterEach = options.afterEach && options.afterEach.apply(this, arguments); return Promise.resolve(afterEach).then(() => destroyApp(this.application)); }, + + after() { + if (options.after) { + return options.after.apply(this, arguments); + } + }, + + before() { + if (options.before) { + return options.before.apply(this, arguments); + } + }, }); } diff --git a/ui/tests/helpers/module-for-job.js b/ui/tests/helpers/module-for-job.js index 3b53c7b6d878..469894162194 100644 --- a/ui/tests/helpers/module-for-job.js +++ b/ui/tests/helpers/module-for-job.js @@ -2,10 +2,17 @@ import { test } from 'qunit'; import moduleForAcceptance from 'nomad-ui/tests/helpers/module-for-acceptance'; import JobDetail from 'nomad-ui/tests/pages/jobs/detail'; -export default function moduleForJob(title, jobFactory, additionalTests) { +export default function moduleForJob(title, context, jobFactory, additionalTests) { let job; moduleForAcceptance(title, { + before() { + if (context !== 'allocations' && context !== 'children') { + throw new Error( + `Invalid context provided to moduleForJob, expected either "allocations" or "children", got ${context}` + ); + } + }, beforeEach() { server.create('node'); job = jobFactory(); @@ -45,6 +52,23 @@ export default function moduleForJob(title, jobFactory, additionalTests) { }); }); + if (context === 'allocations') { + test('allocations for the job are shown in the overview', function(assert) { + assert.ok(JobDetail.allocationsSummary, 'Allocations are shown in the summary section'); + assert.notOk(JobDetail.childrenSummary, 'Children are not shown in the summary section'); + }); + } + + if (context === 'children') { + test('children for the job are shown in the overview', function(assert) { + assert.ok(JobDetail.childrenSummary, 'Children are shown in the summary section'); + assert.notOk( + JobDetail.allocationsSummary, + 'Allocations are not shown in the summary section' + ); + }); + } + for (var testName in additionalTests) { test(testName, function(assert) { additionalTests[testName](job, assert); diff --git a/ui/tests/pages/jobs/detail.js b/ui/tests/pages/jobs/detail.js index 0f5d21bdcc07..f7676811103e 100644 --- a/ui/tests/pages/jobs/detail.js +++ b/ui/tests/pages/jobs/detail.js @@ -31,6 +31,9 @@ export default create({ return this.stats.toArray().findBy('id', id); }, + childrenSummary: isPresent('[data-test-job-summary] [data-test-children-status-bar]'), + allocationsSummary: isPresent('[data-test-job-summary] [data-test-allocation-status-bar]'), + ...allocations(), viewAllAllocations: text('[data-test-view-all-allocations]'),