Skip to content

Commit

Permalink
feat(frontend) Support LIST, STRUCT type in RuntimeConfig parameters (#…
Browse files Browse the repository at this point in the history 
…7991)

* Convert string-type user input to real-type pararmeter. (Currently supporting num, bool, str)

* Delete unnecessary console.log

* Change incorrect type.
Add TODO
Move inputConvert out from function component.
Check parameter type by PipelineSpec before converting.

* Format.

* Move inputConverter out from function component.
Delete unnecessary console.log

* Change inputConverter from if-else to switch.
Assign invalid input to null.

* 1. Change paramTypeIdx to ParameterTypeEnum.
2. Add tests to validate the changes.

* Format

* Support LIST and STRUCT type.

* 1. Fixed default value bugs.
2. Added unit test for LIST and STRUCT type parameters.

* Added type into param.

* Added comment.

* Added unit test for invalid JSON form input.

* Removed incorrect merge conflicts.

* Support double type.
Added and modified unit tests.

* Clean unnecessary expect() in the unit tests.

* Added additional tests to check for the existence of display value 4.56
  • Loading branch information
@jlyaoyuli
jlyaoyuli authored Jul 8, 2022
1 parent 7923ba3 commit 2d81e7b
Show file tree
Hide file tree
Showing 2 changed files with 281 additions and 29 deletions.
233 changes: 218 additions & 15 deletions frontend/src/components/NewRunParametersV2.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ describe('NewRunParametersV2', () => {
</CommonTestWrapper>,
);

expect(screen.getByText('Run parameters'));
expect(screen.getByText('Specify parameters required by the pipeline'));
expect(screen.getByText('strParam - STRING'));
expect(screen.getByDisplayValue('string value'));
expect(screen.getByText('boolParam - BOOLEAN'));
expect(screen.getByDisplayValue('true'));
expect(screen.getByText('intParam - NUMBER_INTEGER'));
expect(screen.getByDisplayValue('123'));
screen.getByText('Run parameters');
screen.getByText('Specify parameters required by the pipeline');
screen.getByText('strParam - STRING');
screen.getByDisplayValue('string value');
screen.getByText('boolParam - BOOLEAN');
screen.getByDisplayValue('true');
screen.getByText('intParam - NUMBER_INTEGER');
screen.getByDisplayValue('123');
});

it('edits parameters', () => {
Expand Down Expand Up @@ -116,6 +116,7 @@ describe('NewRunParametersV2', () => {
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
strParam: 'new string',
});
screen.getByDisplayValue('new string');
});

it('test convertInput function for string type without default value', () => {
Expand All @@ -131,14 +132,15 @@ describe('NewRunParametersV2', () => {
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props}></NewRunParametersV2>);
render(<NewRunParametersV2 {...props} />);

const strParam = screen.getByLabelText('strParam - STRING');
fireEvent.change(strParam, { target: { value: 'new string' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
strParam: 'new string',
});
screen.getByDisplayValue('new string');
});

it('test convertInput function for boolean type with default value', () => {
Expand All @@ -155,14 +157,15 @@ describe('NewRunParametersV2', () => {
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props}></NewRunParametersV2>);
render(<NewRunParametersV2 {...props} />);

const boolParam = screen.getByDisplayValue('true');
fireEvent.change(boolParam, { target: { value: 'false' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
boolParam: false,
});
screen.getByDisplayValue('false');
});

it('test convertInput function for boolean type without default value', () => {
Expand All @@ -178,14 +181,15 @@ describe('NewRunParametersV2', () => {
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props}></NewRunParametersV2>);
render(<NewRunParametersV2 {...props} />);

const boolParam = screen.getByLabelText('boolParam - BOOLEAN');
fireEvent.change(boolParam, { target: { value: 'true' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
boolParam: true,
});
screen.getByDisplayValue('true');
});

it('test convertInput function for boolean type with invalid input (Uppercase)', () => {
Expand All @@ -201,14 +205,15 @@ describe('NewRunParametersV2', () => {
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props}></NewRunParametersV2>);
render(<NewRunParametersV2 {...props} />);

const boolParam = screen.getByLabelText('boolParam - BOOLEAN');
fireEvent.change(boolParam, { target: { value: 'True' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
boolParam: null,
});
screen.getByDisplayValue('True');
});

it('test convertInput function for integer type with default value', () => {
Expand All @@ -225,14 +230,15 @@ describe('NewRunParametersV2', () => {
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props}></NewRunParametersV2>);
render(<NewRunParametersV2 {...props} />);

const intParam = screen.getByDisplayValue('123');
fireEvent.change(intParam, { target: { value: '456' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
intParam: 456,
});
screen.getByDisplayValue('456');
});

it('test convertInput function for integer type without default value', () => {
Expand All @@ -248,14 +254,15 @@ describe('NewRunParametersV2', () => {
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props}></NewRunParametersV2>);
render(<NewRunParametersV2 {...props} />);

const intParam = screen.getByLabelText('intParam - NUMBER_INTEGER');
fireEvent.change(intParam, { target: { value: '789' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
intParam: 789,
});
screen.getByDisplayValue('789');
});

it('test convertInput function for integer type with invalid input (float)', () => {
Expand All @@ -271,14 +278,210 @@ describe('NewRunParametersV2', () => {
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props}></NewRunParametersV2>);
render(<NewRunParametersV2 {...props} />);

const intParam = screen.getByLabelText('intParam - NUMBER_INTEGER');
fireEvent.change(intParam, { target: { value: '7.89' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
intParam: null,
});
screen.getByDisplayValue('7.89');
});

it('test convertInput function for double type with default value', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
doubleParam: {
parameterType: ParameterType_ParameterTypeEnum.NUMBER_DOUBLE,
defaultValue: 1.23,
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const doubleParam = screen.getByDisplayValue('1.23');
fireEvent.change(doubleParam, { target: { value: '4.56' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
doubleParam: 4.56,
});
screen.getByDisplayValue('4.56');
});

it('test convertInput function for double type without default value', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
doubleParam: {
parameterType: ParameterType_ParameterTypeEnum.NUMBER_DOUBLE,
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const doubleParam = screen.getByLabelText('doubleParam - NUMBER_DOUBLE');
fireEvent.change(doubleParam, { target: { value: '7.89' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
doubleParam: 7.89,
});
screen.getByDisplayValue('7.89');
});

it('test convertInput function for LIST type with default value', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
listParam: {
parameterType: ParameterType_ParameterTypeEnum.LIST,
defaultValue: [1, 2, 3],
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const listParam = screen.getByDisplayValue('[1,2,3]');
fireEvent.change(listParam, { target: { value: '[4,5,6]' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
listParam: [4, 5, 6],
});
screen.getByDisplayValue('[4,5,6]');
});

it('test convertInput function for LIST type without default value', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
listParam: {
parameterType: ParameterType_ParameterTypeEnum.LIST,
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const listParam = screen.getByLabelText('listParam - LIST');
fireEvent.change(listParam, { target: { value: '[4,5,6]' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
listParam: [4, 5, 6],
});
screen.getByDisplayValue('[4,5,6]');
});

it('test convertInput function for LIST type with invalid input (invalid JSON form)', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
listParam: {
parameterType: ParameterType_ParameterTypeEnum.LIST,
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const listParam = screen.getByLabelText('listParam - LIST');
fireEvent.change(listParam, { target: { value: '[4,5,6' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
listParam: null,
});
screen.getByDisplayValue('[4,5,6');
});

it('test convertInput function for STRUCT type with default value', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
structParam: {
parameterType: ParameterType_ParameterTypeEnum.STRUCT,
defaultValue: { A: 1, B: 2 },
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const structParam = screen.getByDisplayValue('{"A":1,"B":2}');
fireEvent.change(structParam, { target: { value: '{"C":3,"D":4}' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
structParam: { C: 3, D: 4 },
});
screen.getByDisplayValue('{"C":3,"D":4}');
});

it('test convertInput function for STRUCT type without default value', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
structParam: {
parameterType: ParameterType_ParameterTypeEnum.STRUCT,
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const structParam = screen.getByLabelText('structParam - STRUCT');
fireEvent.change(structParam, { target: { value: '{"A":1,"B":2}' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
structParam: { A: 1, B: 2 },
});
screen.getByDisplayValue('{"A":1,"B":2}');
});

it('test convertInput function for STRUCT type with invalid input (invalid JSON form)', () => {
const handleParameterChangeSpy = jest.fn();
const props = {
titleMessage: 'default Title',
pipelineRoot: 'defalut pipelineRoot',
specParameters: {
structParam: {
parameterType: ParameterType_ParameterTypeEnum.STRUCT,
},
},
handlePipelineRootChange: jest.fn(),
handleParameterChange: handleParameterChangeSpy,
};
render(<NewRunParametersV2 {...props} />);

const structParam = screen.getByLabelText('structParam - STRUCT');
fireEvent.change(structParam, { target: { value: '"A":1,"B":2' } });
expect(handleParameterChangeSpy).toHaveBeenCalledTimes(1);
expect(handleParameterChangeSpy).toHaveBeenLastCalledWith({
structParam: null,
});
screen.getByDisplayValue('"A":1,"B":2');
});

it('does not display any text fields if there are no parameters', () => {
Expand Down
Loading

0 comments on commit 2d81e7b

Please sign in to comment.