diff --git a/ui/app/components/allocation-row.js b/ui/app/components/allocation-row.js index 044d368d2..b210d5b01 100644 --- a/ui/app/components/allocation-row.js +++ b/ui/app/components/allocation-row.js @@ -1,9 +1,11 @@ import Ember from 'ember'; import { lazyClick } from '../helpers/lazy-click'; -const { Component } = Ember; +const { Component, inject } = Ember; export default Component.extend({ + store: inject.service(), + tagName: 'tr', classNames: ['allocation-row', 'is-interactive'], @@ -20,17 +22,35 @@ export default Component.extend({ }, didReceiveAttrs() { + // TODO: Use this code again once the temporary workaround below + // is resolved. + // If the job for this allocation is incomplete, reload it to get // detailed information. + // const allocation = this.get('allocation'); + // if ( + // allocation && + // allocation.get('job') && + // !allocation.get('job.isPending') && + // !allocation.get('taskGroup') + // ) { + // const job = allocation.get('job.content'); + // job && job.reload(); + // } + + // TEMPORARY: https://github.com/emberjs/data/issues/5209 + // Ember Data doesn't like it when relationships aren't reflective, + // which means the allocation's job will be null if it hasn't been + // resolved through the allocation (allocation.get('job')) before + // being resolved through the store (store.findAll('job')). The + // workaround is to persist the jobID as a string on the allocation + // and manually re-link the two records here. + const allocation = this.get('allocation'); - if ( - allocation && - allocation.get('job') && - !allocation.get('job.isPending') && - !allocation.get('taskGroup') - ) { - const job = allocation.get('job.content'); - job && job.reload(); - } + this.get('store') + .findRecord('job', allocation.get('originalJobId')) + .then(job => { + allocation.set('job', job); + }); }, }); diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index ea24a54db..1665980e1 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -19,6 +19,9 @@ export default Model.extend({ resources: fragment('resources'), modifyIndex: attr('number'), + // TEMPORARY: https://github.com/emberjs/data/issues/5209 + originalJobId: attr('string'), + clientStatus: attr('string'), desiredStatus: attr('string'), diff --git a/ui/app/serializers/allocation.js b/ui/app/serializers/allocation.js index 1eec7ec7b..6d46dcaef 100644 --- a/ui/app/serializers/allocation.js +++ b/ui/app/serializers/allocation.js @@ -20,6 +20,9 @@ export default ApplicationSerializer.extend({ return summary; }); + // TEMPORARY: https://github.com/emberjs/data/issues/5209 + hash.OriginalJobId = hash.JobID; + return this._super(typeHash, hash); }, }); diff --git a/ui/tests/acceptance/client-detail-test.js b/ui/tests/acceptance/client-detail-test.js index 01dd007d4..17044134f 100644 --- a/ui/tests/acceptance/client-detail-test.js +++ b/ui/tests/acceptance/client-detail-test.js @@ -16,67 +16,84 @@ moduleForAcceptance('Acceptance | client detail', { server.create('agent'); server.create('job', { createAllocations: false }); server.createList('allocation', 3, { nodeId: node.id }); - - visit(`/nodes/${node.id}`); }, }); test('/nodes/:id should have a breadrcumb trail linking back to nodes', function(assert) { - assert.equal(findAll('.breadcrumb')[0].textContent, 'Nodes', 'First breadcrumb says nodes'); - assert.equal( - findAll('.breadcrumb')[1].textContent, - node.id.split('-')[0], - 'Second breadcrumb says the node short id' - ); + visit(`/nodes/${node.id}`); + + andThen(() => { + assert.equal(findAll('.breadcrumb')[0].textContent, 'Nodes', 'First breadcrumb says nodes'); + assert.equal( + findAll('.breadcrumb')[1].textContent, + node.id.split('-')[0], + 'Second breadcrumb says the node short id' + ); + }); + + andThen(() => { + click(findAll('.breadcrumb')[0]); + }); - click(findAll('.breadcrumb')[0]); andThen(() => { assert.equal(currentURL(), '/nodes', 'First breadcrumb links back to nodes'); }); }); test('/nodes/:id should list immediate details for the node in the title', function(assert) { - assert.ok(find('.title').textContent.includes(node.name), 'Title includes name'); - assert.ok(find('.title').textContent.includes(node.id), 'Title includes id'); - assert.ok( - findAll(`.title .node-status-light.${node.status}`).length, - 'Title includes status light' - ); + visit(`/nodes/${node.id}`); + + andThen(() => { + assert.ok(find('.title').textContent.includes(node.name), 'Title includes name'); + assert.ok(find('.title').textContent.includes(node.id), 'Title includes id'); + assert.ok( + findAll(`.title .node-status-light.${node.status}`).length, + 'Title includes status light' + ); + }); }); test('/nodes/:id should list additional detail for the node below the title', function(assert) { - assert.equal( - findAll('.inline-definitions .pair')[0].textContent, - `Status ${node.status}`, - 'Status is in additional details' - ); - assert.ok( - $('.inline-definitions .pair:eq(0) .status-text').hasClass(`node-${node.status}`), - 'Status is decorated with a status class' - ); - assert.equal( - findAll('.inline-definitions .pair')[1].textContent, - `Address ${node.httpAddr}`, - 'Address is in additional detals' - ); - assert.equal( - findAll('.inline-definitions .pair')[2].textContent, - `Datacenter ${node.datacenter}`, - 'Datacenter is in additional details' - ); + visit(`/nodes/${node.id}`); + + andThen(() => { + assert.equal( + findAll('.inline-definitions .pair')[0].textContent, + `Status ${node.status}`, + 'Status is in additional details' + ); + assert.ok( + $('.inline-definitions .pair:eq(0) .status-text').hasClass(`node-${node.status}`), + 'Status is decorated with a status class' + ); + assert.equal( + findAll('.inline-definitions .pair')[1].textContent, + `Address ${node.httpAddr}`, + 'Address is in additional detals' + ); + assert.equal( + findAll('.inline-definitions .pair')[2].textContent, + `Datacenter ${node.datacenter}`, + 'Datacenter is in additional details' + ); + }); }); test('/nodes/:id should list all allocations on the node', function(assert) { const allocationsCount = server.db.allocations.where({ nodeId: node.id }).length; - assert.equal( - findAll('.allocations tbody tr').length, - allocationsCount, - `Allocations table lists all ${allocationsCount} associated allocations` - ); + + visit(`/nodes/${node.id}`); + + andThen(() => { + assert.equal( + findAll('.allocations tbody tr').length, + allocationsCount, + `Allocations table lists all ${allocationsCount} associated allocations` + ); + }); }); test('each allocation should have high-level details for the allocation', function(assert) { - const allocationRow = $(findAll('.allocations tbody tr')[0]); const allocation = server.db.allocations .where({ nodeId: node.id }) .sortBy('modifyIndex') @@ -90,61 +107,109 @@ test('each allocation should have high-level details for the allocation', functi const tasks = taskGroup.taskIds.map(id => server.db.tasks.find(id)); - assert.equal( - allocationRow - .find('td:eq(0)') - .text() - .trim(), - allocation.id.split('-')[0], - 'Allocation short ID' - ); - assert.equal( - allocationRow - .find('td:eq(1)') - .text() - .trim(), - allocation.name, - 'Allocation name' - ); - assert.equal( - allocationRow - .find('td:eq(2)') - .text() - .trim(), - allocation.clientStatus, - 'Client status' - ); - assert.ok( - allocationRow - .find('td:eq(3)') - .text() - .includes(server.db.jobs.find(allocation.jobId).name), - 'Job name' - ); - assert.ok( - allocationRow - .find('td:eq(3) .is-faded') - .text() - .includes(allocation.taskGroup), - 'Task group name' - ); - assert.equal( - allocationRow - .find('td:eq(4)') - .text() - .trim(), - allocStats.resourceUsage.CpuStats.Percent, - 'CPU %' - ); - assert.equal( - allocationRow - .find('td:eq(5)') - .text() - .trim(), - allocStats.resourceUsage.MemoryStats.Cache / - tasks.reduce((sum, task) => sum + task.Resources.MemoryMB, 0), - 'Memory used' - ); + visit(`/nodes/${node.id}`); + + andThen(() => { + const allocationRow = $(findAll('.allocations tbody tr')[0]); + assert.equal( + allocationRow + .find('td:eq(0)') + .text() + .trim(), + allocation.id.split('-')[0], + 'Allocation short ID' + ); + assert.equal( + allocationRow + .find('td:eq(1)') + .text() + .trim(), + allocation.name, + 'Allocation name' + ); + assert.equal( + allocationRow + .find('td:eq(2)') + .text() + .trim(), + allocation.clientStatus, + 'Client status' + ); + assert.ok( + allocationRow + .find('td:eq(3)') + .text() + .includes(server.db.jobs.find(allocation.jobId).name), + 'Job name' + ); + assert.ok( + allocationRow + .find('td:eq(3) .is-faded') + .text() + .includes(allocation.taskGroup), + 'Task group name' + ); + assert.equal( + allocationRow + .find('td:eq(4)') + .text() + .trim(), + allocStats.resourceUsage.CpuStats.Percent, + 'CPU %' + ); + assert.equal( + allocationRow + .find('td:eq(5)') + .text() + .trim(), + allocStats.resourceUsage.MemoryStats.Cache / + tasks.reduce((sum, task) => sum + task.Resources.MemoryMB, 0), + 'Memory used' + ); + }); +}); + +test('each allocation should show job information even if the job is incomplete and already in the store', function( + assert +) { + // First, visit nodes to load the allocations for each visible node. + // Don't load the job belongsTo of the allocation! Leave it unfulfilled. + + visit('/nodes'); + + // Then, visit jobs to load all jobs, which should implicitly fulfill + // the job belongsTo of each allocation pointed at each job. + + visit('/jobs'); + + // Finally, visit a node to assert that the job name and task group name are + // present. This will require reloading the job, since task groups aren't a + // part of the jobs list response. + + visit(`/nodes/${node.id}`); + + andThen(() => { + const allocationRow = $(findAll('.allocations tbody tr')[0]); + const allocation = server.db.allocations + .where({ nodeId: node.id }) + .sortBy('modifyIndex') + .reverse()[0]; + + assert.ok( + allocationRow + .find('td:eq(3)') + .text() + .includes(server.db.jobs.find(allocation.jobId).name), + 'Job name' + ); + assert.ok( + allocationRow + .find('td:eq(3) .is-faded') + .text() + .includes(allocation.taskGroup), + 'Task group name' + ); + }); }); test('each allocation should link to the allocation detail page', function(assert) { @@ -153,7 +218,11 @@ test('each allocation should link to the allocation detail page', function(asser .sortBy('modifyIndex') .reverse()[0]; - click($('.allocations tbody tr:eq(0) td:eq(0) a').get(0)); + visit(`/nodes/${node.id}`); + + andThen(() => { + click($('.allocations tbody tr:eq(0) td:eq(0) a').get(0)); + }); andThen(() => { assert.equal( @@ -165,9 +234,14 @@ test('each allocation should link to the allocation detail page', function(asser }); test('each allocation should link to the job the allocation belongs to', function(assert) { + visit(`/nodes/${node.id}`); + const allocation = server.db.allocations.where({ nodeId: node.id })[0]; const job = server.db.jobs.find(allocation.jobId); - click($('.allocations tbody tr:eq(0) td:eq(3) a').get(0)); + + andThen(() => { + click($('.allocations tbody tr:eq(0) td:eq(3) a').get(0)); + }); andThen(() => { assert.equal( @@ -179,7 +253,11 @@ test('each allocation should link to the job the allocation belongs to', functio }); test('/nodes/:id should list all attributes for the node', function(assert) { - assert.ok(find('.attributes-table'), 'Attributes table is on the page'); + visit(`/nodes/${node.id}`); + + andThen(() => { + assert.ok(find('.attributes-table'), 'Attributes table is on the page'); + }); }); test('when the node is not found, an error message is shown, but the URL persists', function(