[CCR] Improve remote clusters test coverage

[cjcenizal]

I added tests for:

• RemoteClusterForm validation state
• Individual RemoteCluster validators
• RemoteClusterTable and its different row states and row button behavior
• DetailPanel and its button behavior
• RemoteClusterList empty prompt

[elasticmachine]

Pinging @elastic/es-ui

[cjcenizal]

@elastic/es-ui I was looking at the code duplication here and I was thinking we could apply a page object model to this code. What do you all think? If you think this direction has merit I can submit a PR to allow us to all improve on it together, and then could apply it to ILM's tests, which gave me the original idea. A page object would act as an abstraction over our data-test-subj selectors so we could change their names without having to make updates to multiple test files, and theoretically we could use them in multiple tests throughout our UI tree.

Here's sort of what I was thinking:

// remote_cluster_table.page_object.js
export class RemoteClusterTablePageObject {
  constructor(props, store) {
    this.component = mountWithIntl(
      <Provider store={store}>
        <RemoteClusterTable
          { ...props }
        />
      </Provider>
    );
  }

  getRowNameLink(name) {
    return findTestSubject(this.component, `remoteClusterTableRowName-${name}`);
  }

  getRowDeleteButton(name) {
    return findTestSubject(this.component, `remoteClusterTableRowRemoveButton-${name}`);
  }

  getRowEditButton(name) {
    return findTestSubject(this.component, `remoteClusterTableRowEditButton-${name}`);
  }

  getDetailPanel() {
    return findTestSubject(this.component, 'remoteClusterDetailFlyout');
  }

  getConfirmDeleteModal() {
    return findTestSubject(this.component, 'remoteClustersDeleteConfirmModal');
  }
}

// example of consumption in test.js
const pageObject = new RemoteClusterTablePageObject({
  clusters,
  openDetailPanel: () => {},
});

const rowName = pageObject.getRowNameLink('test-cluster');
rowName.simulate('click');
const detailPanel = pageObject.getDetailPanel();
expect(detailPanel).toBeTruthy();

[sebelga]

I am not sure I follow you. Maybe we can talk about it on Zoom so I can see the benefit.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[cjcenizal]

Known failure:

X-Pack Chromium API Reporting Tests.x-pack/test/reporting/api/bwc_existing_indexes·js.chromium BWC report generation into existing indexes existing 6_2 index multiple jobs posted

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[sebelga]

LGTM! Just a few code style comments. 😊

[sebelga]

The positive is clearer to me: const isValid = isSeedNodeValid(seed) and then if (!isValid) ...

[sebelga]

Same here.

[sebelga]

IMO it makes more sense that a validator function return either an error or null.

const error = validateSeeds(<...args>); // null

undefined seems like it hasn't been validated.

[sebelga]

I would have probably put the component instantiation inside a beforeEach to avoid duplication in both tests.

[cjcenizal]

I think it's incidental that they happen to instantiate with identical state, so I'd prefer to avoid the coupling introduced by a beforeEach here.

[cjcenizal]

On second thought, because we're testing row actions it makes sense to couple the tests with a beforeEach and in fact, they're already coupled by the clusters variable.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request