From f543137ec989149922b76135f6f4b105108f9fdb Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 1 Sep 2020 09:49:36 -0700 Subject: [PATCH 1/2] Revert "Temporarily remove poststop from the UI" This reverts commit f8a505ea2f9def2a7ce7a93e6d0529e1b5da4894. --- ui/app/components/lifecycle-chart.js | 5 +++++ ui/app/styles/components/lifecycle-chart.scss | 21 +++++++++++++------ .../templates/components/lifecycle-chart.hbs | 3 +++ ui/mirage/factories/task.js | 4 +++- ui/tests/acceptance/allocation-detail-test.js | 3 +-- ui/tests/acceptance/task-detail-test.js | 9 ++------ .../components/lifecycle-chart-test.js | 16 ++++++++++---- ui/tests/pages/components/lifecycle-chart.js | 1 + 8 files changed, 42 insertions(+), 20 deletions(-) diff --git a/ui/app/components/lifecycle-chart.js b/ui/app/components/lifecycle-chart.js index 602fdd674..a58e6b839 100644 --- a/ui/app/components/lifecycle-chart.js +++ b/ui/app/components/lifecycle-chart.js @@ -47,6 +47,11 @@ export default class LifecycleChart extends Component { phases.push({ name: 'Poststart', }); + + phases.push({ + name: 'Poststop', + isActive: lifecycles.poststops.some(stateActiveIterator), + }); } return phases; diff --git a/ui/app/styles/components/lifecycle-chart.scss b/ui/app/styles/components/lifecycle-chart.scss index b983f75d5..aec38bd91 100644 --- a/ui/app/styles/components/lifecycle-chart.scss +++ b/ui/app/styles/components/lifecycle-chart.scss @@ -33,6 +33,7 @@ position: absolute; bottom: 0; top: 0; + border-top: 2px solid transparent; .name { @@ -58,12 +59,12 @@ &.main { left: 25%; - right: 0; + right: 25%; } &.poststart { - left: 40%; - right: 0; + left: 35%; + right: 25%; } &.poststop { @@ -126,20 +127,28 @@ &.main { margin-left: 25%; - margin-right: 0; + margin-right: 25%; } &.prestart-ephemeral { margin-right: 75%; } + &.prestart-sidecar { + margin-right: 25%; + } + &.poststart-ephemeral, &.poststart-sidecar { - margin-left: 40%; + margin-left: 35%; + } + + &.poststart-sidecar { + margin-right: 25%; } &.poststart-ephemeral { - margin-right: 10%; + margin-right: 35%; } &.poststop { diff --git a/ui/app/templates/components/lifecycle-chart.hbs b/ui/app/templates/components/lifecycle-chart.hbs index f15141123..5cf17a9b1 100644 --- a/ui/app/templates/components/lifecycle-chart.hbs +++ b/ui/app/templates/components/lifecycle-chart.hbs @@ -14,6 +14,9 @@ + + +
diff --git a/ui/mirage/factories/task.js b/ui/mirage/factories/task.js index 10d688e0f..88c5b23cf 100644 --- a/ui/mirage/factories/task.js +++ b/ui/mirage/factories/task.js @@ -31,7 +31,7 @@ export default Factory.extend({ }, Lifecycle: i => { - const cycle = i % 5; + const cycle = i % 6; if (cycle === 0) { return null; @@ -43,6 +43,8 @@ export default Factory.extend({ return { Hook: 'poststart', Sidecar: false }; } else if (cycle === 4) { return { Hook: 'poststart', Sidecar: true }; + } else if (cycle === 5) { + return { Hook: 'poststop' }; } }, diff --git a/ui/tests/acceptance/allocation-detail-test.js b/ui/tests/acceptance/allocation-detail-test.js index 83fa23898..d9c938f53 100644 --- a/ui/tests/acceptance/allocation-detail-test.js +++ b/ui/tests/acceptance/allocation-detail-test.js @@ -96,14 +96,13 @@ module('Acceptance | allocation detail', function(hooks) { assert.ok(Allocation.lifecycleChart.isPresent); assert.equal(Allocation.lifecycleChart.title, 'Task Lifecycle Status'); - assert.equal(Allocation.lifecycleChart.phases.length, 3); + assert.equal(Allocation.lifecycleChart.phases.length, 4); assert.equal(Allocation.lifecycleChart.tasks.length, 6); await Allocation.lifecycleChart.tasks[0].visit(); const prestartEphemeralTask = server.db.taskStates .where({ allocationId: allocation.id }) - .sortBy('name') .find(taskState => { const task = server.db.tasks.findBy({ name: taskState.name }); return task.Lifecycle && task.Lifecycle.Hook === 'prestart' && !task.Lifecycle.Sidecar; diff --git a/ui/tests/acceptance/task-detail-test.js b/ui/tests/acceptance/task-detail-test.js index 7a9b4390a..32b9c11cd 100644 --- a/ui/tests/acceptance/task-detail-test.js +++ b/ui/tests/acceptance/task-detail-test.js @@ -37,13 +37,8 @@ module('Acceptance | task detail', function(hooks) { ); const lifecycle = server.db.tasks.where({ name: task.name })[0].Lifecycle; - - let lifecycleName = 'main'; - if (lifecycle && (lifecycle.Hook === 'prestart' || lifecycle.Hook === 'poststart')) { - lifecycleName = `${lifecycle.Hook}-${lifecycle.Sidecar ? 'sidecar' : 'ephemeral'}`; - } - - assert.equal(Task.lifecycle, lifecycleName); + const prestartString = lifecycle && lifecycle.Sidecar ? 'sidecar' : 'prestart'; + assert.equal(Task.lifecycle, lifecycle ? prestartString : 'main'); assert.equal(document.title, `Task ${task.name} - Nomad`); }); diff --git a/ui/tests/integration/components/lifecycle-chart-test.js b/ui/tests/integration/components/lifecycle-chart-test.js index 90d65abca..864f3fcfb 100644 --- a/ui/tests/integration/components/lifecycle-chart-test.js +++ b/ui/tests/integration/components/lifecycle-chart-test.js @@ -34,6 +34,10 @@ const tasks = [ lifecycleName: 'poststart-sidecar', name: 'poststart sidecar: 4', }, + { + lifecycleName: 'poststop', + name: 'poststop: 6', + }, ]; module('Integration | Component | lifecycle-chart', function(hooks) { @@ -48,6 +52,7 @@ module('Integration | Component | lifecycle-chart', function(hooks) { assert.equal(Chart.phases[0].name, 'Prestart'); assert.equal(Chart.phases[1].name, 'Main'); assert.equal(Chart.phases[2].name, 'Poststart'); + assert.equal(Chart.phases[3].name, 'Poststop'); Chart.phases.forEach(phase => assert.notOk(phase.isActive)); @@ -58,6 +63,7 @@ module('Integration | Component | lifecycle-chart', function(hooks) { 'main two: 3', 'poststart sidecar: 4', 'poststart ephemeral: 5', + 'poststop: 6', ]); assert.deepEqual(Chart.tasks.mapBy('lifecycle'), [ 'Prestart Task', @@ -66,6 +72,7 @@ module('Integration | Component | lifecycle-chart', function(hooks) { 'Main Task', 'Sidecar Task', 'Poststart Task', + 'Poststop Task', ]); assert.ok(Chart.tasks[0].isPrestartEphemeral); @@ -73,6 +80,7 @@ module('Integration | Component | lifecycle-chart', function(hooks) { assert.ok(Chart.tasks[2].isMain); assert.ok(Chart.tasks[4].isPoststartSidecar); assert.ok(Chart.tasks[5].isPoststartEphemeral); + assert.ok(Chart.tasks[6].isPoststop); Chart.tasks.forEach(task => { assert.notOk(task.isActive); @@ -94,10 +102,10 @@ module('Integration | Component | lifecycle-chart', function(hooks) { }); test('it renders all phases when there are any non-main tasks', async function(assert) { - this.set('tasks', [tasks[0], tasks[2]]); + this.set('tasks', [tasks[0], tasks[6]]); await render(hbs``); - assert.equal(Chart.phases.length, 3); + assert.ok(Chart.phases.length, 4); }); test('it reflects phase and task states when states are passed in', async function(assert) { @@ -141,8 +149,8 @@ module('Integration | Component | lifecycle-chart', function(hooks) { [ { testName: 'expected active phases', - runningTaskNames: ['prestart ephemeral', 'main one'], - activePhaseNames: ['Prestart', 'Main'], + runningTaskNames: ['prestart ephemeral', 'main one', 'poststop'], + activePhaseNames: ['Prestart', 'Main', 'Poststop'], }, { testName: 'sidecar task states don’t affect phase active states', diff --git a/ui/tests/pages/components/lifecycle-chart.js b/ui/tests/pages/components/lifecycle-chart.js index 68781845d..47f1493e4 100644 --- a/ui/tests/pages/components/lifecycle-chart.js +++ b/ui/tests/pages/components/lifecycle-chart.js @@ -23,6 +23,7 @@ export default { isPrestartSidecar: hasClass('prestart-sidecar'), isPoststartEphemeral: hasClass('poststart-ephemeral'), isPoststartSidecar: hasClass('poststart-sidecar'), + isPoststop: hasClass('poststop'), visit: clickable('a'), }), From c041236629cb3eeb65dc7e1888a78e59357ff63c Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 1 Sep 2020 10:02:10 -0700 Subject: [PATCH 2/2] Fix flaky lifecycle chart tests --- ui/tests/acceptance/allocation-detail-test.js | 1 + ui/tests/acceptance/task-detail-test.js | 12 ++++++++++-- .../integration/components/lifecycle-chart-test.js | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/ui/tests/acceptance/allocation-detail-test.js b/ui/tests/acceptance/allocation-detail-test.js index d9c938f53..bb68f394e 100644 --- a/ui/tests/acceptance/allocation-detail-test.js +++ b/ui/tests/acceptance/allocation-detail-test.js @@ -103,6 +103,7 @@ module('Acceptance | allocation detail', function(hooks) { const prestartEphemeralTask = server.db.taskStates .where({ allocationId: allocation.id }) + .sortBy('name') .find(taskState => { const task = server.db.tasks.findBy({ name: taskState.name }); return task.Lifecycle && task.Lifecycle.Hook === 'prestart' && !task.Lifecycle.Sidecar; diff --git a/ui/tests/acceptance/task-detail-test.js b/ui/tests/acceptance/task-detail-test.js index 32b9c11cd..3e54d44a1 100644 --- a/ui/tests/acceptance/task-detail-test.js +++ b/ui/tests/acceptance/task-detail-test.js @@ -37,8 +37,16 @@ module('Acceptance | task detail', function(hooks) { ); const lifecycle = server.db.tasks.where({ name: task.name })[0].Lifecycle; - const prestartString = lifecycle && lifecycle.Sidecar ? 'sidecar' : 'prestart'; - assert.equal(Task.lifecycle, lifecycle ? prestartString : 'main'); + + let lifecycleName = 'main'; + if (lifecycle && (lifecycle.Hook === 'prestart' || lifecycle.Hook === 'poststart')) { + lifecycleName = `${lifecycle.Hook}-${lifecycle.Sidecar ? 'sidecar' : 'ephemeral'}`; + } + if (lifecycle && lifecycle.Hook === 'poststop') { + lifecycleName = 'poststop'; + } + + assert.equal(Task.lifecycle, lifecycleName); assert.equal(document.title, `Task ${task.name} - Nomad`); }); diff --git a/ui/tests/integration/components/lifecycle-chart-test.js b/ui/tests/integration/components/lifecycle-chart-test.js index 864f3fcfb..26032169a 100644 --- a/ui/tests/integration/components/lifecycle-chart-test.js +++ b/ui/tests/integration/components/lifecycle-chart-test.js @@ -105,7 +105,7 @@ module('Integration | Component | lifecycle-chart', function(hooks) { this.set('tasks', [tasks[0], tasks[6]]); await render(hbs``); - assert.ok(Chart.phases.length, 4); + assert.equal(Chart.phases.length, 4); }); test('it reflects phase and task states when states are passed in', async function(assert) {