Skip to content

Commit

Permalink
[ML] Data Frame Analytics creation: ensure form state persists after …
Browse files Browse the repository at this point in the history
…switch to editor (elastic#169186)

## Summary

This PR:
- ensures the form state is correctly persisted when switching back to
form state from the editor
- fixes the issue where the 'Create index pattern' checkbox would no
longer be checked by default when switching back from the editor
- adds functional tests for the data frame analytics wizard covering the
case of switching to json editor and back to form

Flaky test runner
[build](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3558)
✅ 100/100 runs passed


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
alvarezmelissa87 and kibanamachine authored Oct 19, 2023
1 parent 1fcdd34 commit 27fcb77
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export const AdvancedStepForm: FC<CreateAnalyticsStepProps> = ({
randomizeSeed,
softTreeDepthLimit,
softTreeDepthTolerance,
useEstimatedMml,
]);

const outlierDetectionAdvancedConfig = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,38 +139,40 @@ export const CreateAnalyticsAdvancedEditor: FC<CreateAnalyticsFormProps> = (prop
)}
style={{ maxWidth: '100%' }}
>
<CodeEditor
languageId={'json'}
height={500}
languageConfiguration={{
autoClosingPairs: [
{
open: '{',
close: '}',
<div data-test-subj={'mlAnalyticsCreateJobWizardAdvancedEditorCodeEditor'}>
<CodeEditor
languageId={'json'}
height={500}
languageConfiguration={{
autoClosingPairs: [
{
open: '{',
close: '}',
},
],
}}
value={advancedEditorRawString}
onChange={onChange}
options={{
ariaLabel: i18n.translate(
'xpack.ml.dataframe.analytics.create.advancedEditor.codeEditorAriaLabel',
{
defaultMessage: 'Advanced analytics job editor',
}
),
automaticLayout: true,
readOnly: isJobCreated,
fontSize: 12,
scrollBeyondLastLine: false,
quickSuggestions: true,
minimap: {
enabled: false,
},
],
}}
value={advancedEditorRawString}
onChange={onChange}
options={{
ariaLabel: i18n.translate(
'xpack.ml.dataframe.analytics.create.advancedEditor.codeEditorAriaLabel',
{
defaultMessage: 'Advanced analytics job editor',
}
),
automaticLayout: true,
readOnly: isJobCreated,
fontSize: 12,
scrollBeyondLastLine: false,
quickSuggestions: true,
minimap: {
enabled: false,
},
wordWrap: 'on',
wrappingIndent: 'indent',
}}
/>
wordWrap: 'on',
wrappingIndent: 'indent',
}}
/>
</div>
</EuiFormRow>
<EuiSpacer />
{advancedEditorMessages.map((advancedEditorMessage, i) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,9 @@ export function reducer(state: State, action: Action): State {
const { jobConfig: config } = state;
const { jobId } = state.form;
// @ts-ignore
const formState = getFormStateFromJobConfig(config, false);
const formStateFromJobConfig = getFormStateFromJobConfig(config, false);
// Ensure previous form settings are persisted. Form state does not include any nested attributes.
const formState = { ...formStateFromJobConfig, ...state.form };

if (typeof jobId === 'string' && jobId.trim() !== '') {
formState.jobId = jobId;
Expand Down Expand Up @@ -605,7 +607,6 @@ export function reducer(state: State, action: Action): State {

return validateForm({
...state,
// @ts-ignore
form: formState,
isAdvancedEditorEnabled: false,
advancedEditorRawString: JSON.stringify(config, null, 2),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ export default function ({ getService }: FtrProviderContext) {
isDependentVariableInput: true,
},
],
advancedEditorContent: [
'{',
` "description": "Classification job based on 'ft_bank_marketing' dataset with dependentVariable 'y' and trainingPercent '20'",`,
' "source": {',
],
expected: {
rocCurveColorState: [
// tick/grid/axis
Expand Down Expand Up @@ -321,6 +326,14 @@ export default function ({ getService }: FtrProviderContext) {
// - ⚠ Analysis fields
await ml.dataFrameAnalyticsCreation.assertAllValidationCalloutsPresent(4);

// switch to json editor and back
await ml.testExecution.logTestStep('switches to advanced editor then back to form');
await ml.dataFrameAnalyticsCreation.openAdvancedEditor();
await ml.dataFrameAnalyticsCreation.assertAdvancedEditorCodeEditorContent(
testData.advancedEditorContent
);
await ml.dataFrameAnalyticsCreation.closeAdvancedEditor();

await ml.testExecution.logTestStep('continues to the create step');
await ml.dataFrameAnalyticsCreation.continueToCreateStep();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ export default function ({ getService }: FtrProviderContext) {
},
modelMemory: '5mb',
createIndexPattern: true,
advancedEditorContent: [
'{',
' "description": "Outlier detection job based on ft_ihp_outlier dataset with runtime fields",',
' "source": {',
],
expected: {
histogramCharts: [
{ chartAvailable: true, id: '1stFlrSF', legend: '334 - 4692' },
Expand Down Expand Up @@ -307,6 +312,14 @@ export default function ({ getService }: FtrProviderContext) {
await ml.dataFrameAnalyticsCreation.assertValidationCalloutsExists();
await ml.dataFrameAnalyticsCreation.assertAllValidationCalloutsPresent(1);

// switch to json editor and back
await ml.testExecution.logTestStep('switches to advanced editor then back to form');
await ml.dataFrameAnalyticsCreation.openAdvancedEditor();
await ml.dataFrameAnalyticsCreation.assertAdvancedEditorCodeEditorContent(
testData.advancedEditorContent
);
await ml.dataFrameAnalyticsCreation.closeAdvancedEditor();

await ml.testExecution.logTestStep('continues to the create step');
await ml.dataFrameAnalyticsCreation.continueToCreateStep();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ export default function ({ getService }: FtrProviderContext) {
trainingPercent: 20,
modelMemory: '20mb',
createIndexPattern: true,
advancedEditorContent: [
'{',
' "description": "Regression job based on ft_egs_regression dataset with runtime fields",',
' "source": {',
],
expected: {
scatterplotMatrixColorStats: [
// some marker colors of the continuous color scale
Expand Down Expand Up @@ -322,6 +327,14 @@ export default function ({ getService }: FtrProviderContext) {
await ml.dataFrameAnalyticsCreation.assertValidationCalloutsExists();
await ml.dataFrameAnalyticsCreation.assertAllValidationCalloutsPresent(3);

// switch to json editor and back
await ml.testExecution.logTestStep('switches to advanced editor then back to form');
await ml.dataFrameAnalyticsCreation.openAdvancedEditor();
await ml.dataFrameAnalyticsCreation.assertAdvancedEditorCodeEditorContent(
testData.advancedEditorContent
);
await ml.dataFrameAnalyticsCreation.closeAdvancedEditor();

await ml.testExecution.logTestStep('continues to the create step');
await ml.dataFrameAnalyticsCreation.continueToCreateStep();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,40 @@ export function MachineLearningDataFrameAnalyticsCreationProvider(
await headerPage.waitUntilLoadingHasFinished();
},

async assertAdvancedEditorCodeEditorExists() {
await testSubjects.existOrFail('mlAnalyticsCreateJobWizardAdvancedEditorCodeEditor', {
allowHidden: true,
});
},

async assertAdvancedEditorCodeEditorContent(expectedContent: string[]) {
await this.assertAdvancedEditorCodeEditorExists();
const wrapper = await testSubjects.find('mlAnalyticsCreateJobWizardAdvancedEditorCodeEditor');
const editor = await wrapper.findByCssSelector('.monaco-editor .view-lines');
const editorContentString = await editor.getVisibleText();
const splicedAdvancedEditorValue = editorContentString.split('\n').splice(0, 3);
expect(splicedAdvancedEditorValue).to.eql(
expectedContent,
`Expected the first editor lines to be '${expectedContent}' (got '${splicedAdvancedEditorValue}')`
);
},

async openAdvancedEditor() {
this.assertAdvancedEditorSwitchExists();
await testSubjects.click('mlAnalyticsCreateJobWizardAdvancedEditorSwitch');
this.assertAdvancedEditorSwitchCheckState(true);
this.assertAdvancedEditorCodeEditorExists();
},

async closeAdvancedEditor() {
this.assertAdvancedEditorSwitchExists();
await testSubjects.click('mlAnalyticsCreateJobWizardAdvancedEditorSwitch');
this.assertAdvancedEditorSwitchCheckState(false);
await testSubjects.missingOrFail('mlAnalyticsCreateJobWizardAdvancedEditorCodeEditor');
},

async assertAdvancedEditorSwitchExists() {
await testSubjects.existOrFail(`mlAnalyticsCreateJobWizardAdvancedEditorSwitch`, {
await testSubjects.existOrFail('mlAnalyticsCreateJobWizardAdvancedEditorSwitch', {
allowHidden: true,
});
},
Expand Down

0 comments on commit 27fcb77

Please sign in to comment.