Skip to content

Commit

Permalink
fix(frontend): Remove alphabetical sorting of the metrics column. Fixes
Browse files Browse the repository at this point in the history
#5215 (#5701)

* Remove alphabetical sorting of the metrics column

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Fix code format

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Refactor test to use data-testid

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>

* Remove snapshot in favor of text comparison

Signed-off-by: Anna Jung (VMware) <antheaj@vmware.com>
  • Loading branch information
Anna authored May 27, 2021
1 parent fba546e commit ebf5310
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 deletions.
2 changes: 1 addition & 1 deletion frontend/src/components/Metric.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class Metric extends React.PureComponent<MetricProps> {
}
return (
<div className={css.metricContainer}>
<div className={css.metricFill} style={{ width }}>
<div className={css.metricFill} style={{ width }} data-testid={'metric'}>
{displayString}
</div>
</div>
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/components/__snapshots__/Metric.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ exports[`Metric renders a metric and does not log an error when metric is betwee
>
<div
className="metricFill"
data-testid="metric"
style={
Object {
"width": "calc(54%)",
Expand Down Expand Up @@ -59,6 +60,7 @@ exports[`Metric renders a metric when metric has value and percentage format 1`]
>
<div
className="metricFill"
data-testid="metric"
style={
Object {
"width": "calc(54.000%)",
Expand Down
4 changes: 1 addition & 3 deletions frontend/src/lib/RunUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import { orderBy } from 'lodash';
import { ApiParameter, ApiPipelineVersion } from '../apis/pipeline';
import { Workflow } from '../../third_party/argo-ui/argo_template';
import { ApiJob } from '../apis/job';
Expand Down Expand Up @@ -166,8 +165,7 @@ function runsToMetricMetadataMap(runs: ApiRun[]): Map<string, MetricMetadata> {
}

function extractMetricMetadata(runs: ApiRun[]): MetricMetadata[] {
const metrics = Array.from(runsToMetricMetadataMap(runs).values());
return orderBy(metrics, ['count', 'name'], ['desc', 'asc']);
return Array.from(runsToMetricMetadataMap(runs).values());
}

function getRecurringRunId(run?: ApiRun): string {
Expand Down
21 changes: 20 additions & 1 deletion frontend/src/pages/RunList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { MetricMetadata } from '../lib/RunUtils';
import { NodePhase } from '../lib/StatusUtils';
import { ReactWrapper, ShallowWrapper, shallow } from 'enzyme';
import { range } from 'lodash';
import { BrowserRouter } from 'react-router-dom';
import MetricUtils from '../lib/MetricUtils';

class RunListTest extends RunList {
public _loadRuns(request: ListRequest): Promise<string> {
Expand Down Expand Up @@ -398,6 +398,25 @@ describe('RunList', () => {
expect(tree).toMatchSnapshot();
});

it('adds metrics column in the same order metrics were defined', async () => {
const z_metric = { name: 'z_metric', number_value: 2 } as ApiRunMetric;
const y_metric = { name: 'y_metric', number_value: 1 } as ApiRunMetric;

let actualValues = [
MetricUtils.getMetricDisplayString(z_metric),
MetricUtils.getMetricDisplayString(y_metric),
];

mockNRuns(1, { run: { metrics: [z_metric, y_metric] } });
const props = generateProps();
tree = TestUtils.mountWithRouter(<RunList {...props} />);
await TestUtils.flushPromises();
tree.update();

const metricValues = tree.find("[data-testid='metric']");
expect(metricValues.map(v => v.text())).toEqual(actualValues);
});

it('shows pipeline name', async () => {
mockNRuns(1, {
run: { pipeline_spec: { pipeline_id: 'test-pipeline-id', pipeline_name: 'pipeline name' } },
Expand Down

0 comments on commit ebf5310

Please sign in to comment.