[WIP] - Added automated tests with cypress for Zone form

[asirvadAbrahamVarghese]

CP4AIOPS-17892
Pr that adds cypress tests for Zone form: Settings > Application Settings > Settings > Zones > Configuration > Add/Edit Zone
 - Validate add form elements
 - Validate edit form elements
 - Add, edit & delete a zone
 
@miq-bot assign @elsamaryv
@miq-bot add-label cypress
@miq-bot add-label test
 

[asirvadAbrahamVarghese]

TODO:

• Use commands for form elements selection: Add custom commands for selecting form elements #9537 ✅
• Switch to direct API calls for cleanup, if the implementation under [WIP] - Added automated tests with cypress for Namespace form #9517 is approved

[jrafanie]

name, description, save, cancel, add, reset, etc. might make the test more readable if the simple string is used inline in the test...

maxScanSelectFieldId and maxScanSelectFieldId are complicated enough to be worth leaving as constants. I don't really know.

see also: https://github.com/ManageIQ/manageiq-ui-classic/pull/9549/files#r2263776498

[elsamaryv]

I too have the opinion that using ids/strings inline makes it more readable

[asirvadAbrahamVarghese]

I completely agree that input types, button types, and similar HTML values are standard and unlikely to change, so it’s perfectly fine to keep them inline.
For things like button labels or other frequently repeated field names, the goal is to reduce redundancy. That way, if a button label changes - say from “Add” to “Add Zone” we only need to update it in one place - the constants variable (now we have element selector commands(getFormFooterButtonByType) that uses a "contains" check, even “Add” will still match)

Ultimately, the best practice is to maintain a shared constants file for both the component and its test files. This ensures that any updates only need to be made once in that shared file.

Something like this 57ec79fa - which I suggested for the schedule form - this constants file currently holds only IDs, but anything you expect might change in the future such as flash messages, button labels, headers,... can all be included in this

[jrafanie]

yeah, I think we can have a shared set of constants but simple strings unique to the test can probably be done inline.

[asirvadAbrahamVarghese]

Keeping simple strings like name, description inline (see) and keeping IP_INPUT_FIELD_ID and MAX_SCAN_SELECT_FIELD_ID as-is since they include escape sequences

[jrafanie]

Similar to the question above about the input fields type() possibly doing async stuff afterwards... I'm seeing one run of this test retry 4 times. So, don't waste time trying to debug it until this test actually fails all 10 attempts but wanted to share in case there was something obvious async you saw when you ran this manually.

[asirvadAbrahamVarghese]

I think the session timing problem is the culprit here again, and doesn’t look like there’s anything async that’s needs to be awaited or handled

[jrafanie]

haha, I didn't get any takers on reviewing #9533 so it's stalled. Reducing our session writes on requests that shouldn't be updating session doesn't fix the problem but it does reduce the likelihood of a timing bug with much less session writes across threads.

[jrafanie]

Close / open after revert of #9505 via #9553

[jrafanie]

After we do a few pages of these, we'll probably have shared behavior we can extract. It's probably best to get a few of these in first.

[asirvadAbrahamVarghese]

I am not really sure about the best approach to extract these, one way would be to have separate methods in a shared helper file for each field, something like:

function validateNameField(isDisabled = false) {
  cy.getFormLabelByInputId('name').should('be.visible');
  cy.getFormInputFieldById('name')
    .should('be.visible')
    .then((nameField) => {
      if (isDisabled) {
        expect(nameField).to.be.disabled;
      } else {
        expect(nameField).to.not.be.disabled;
      }
    });
}

But we might end up with a pile of methods 😴

More flexible one would be to get the set of field configs and then do the assertions:

function validateFormFields(fieldConfigs) {
  fieldConfigs.forEach((config) => {
    const { id, fieldType = 'input', shouldBeDisabled = false } = config;

    // Check label
    cy.getFormLabelByInputId(id).should('be.visible');

    // Check field
    if (fieldType === 'input') {
      cy.getFormInputFieldById(id)
        .should('be.visible')
        .then((field) => {
          if (shouldBeDisabled) {
            expect(field).to.be.disabled;
          } else {
            expect(field).to.not.be.disabled;
          }
        });
    } else if (fieldType === 'select') {
      cy.getFormSelectFieldById(id)
        .should('be.visible')
        .then((field) => {
          if (shouldBeDisabled) {
            expect(field).to.be.disabled;
          } else {
            expect(field).to.not.be.disabled;
          }
        });
    }
  });
}

and use it like:

validateFormFields([
  { id: 'name', shouldBeDisabled: true },
  { id: 'description' },
  { fieldType: 'select', id: 'concurrent_vm_scans' },
]);

Any thoughts?

[jrafanie]

Is this the same treeSelectApi from the beforeEach? I wonder if this can/should be moved into the selectAccordionItem in the future. Our other functions seem to handle waits for us but this one doesn't. It's stands out to me as odd.

[asirvadAbrahamVarghese]

Is this the same treeSelectApi from the beforeEach?

Moved it to a common method so that cy.wait will be handled using cy.interceptApi command

I wonder if this can/should be moved into the selectAccordionItem in the future.

As of now we can't entirely shift the treeSelect intercept & wait logic into selectAccordionItem via interceptApi (as of now this will only handle duplicate aliasing) due to mixed behavior with accordion selection.
Sometimes, reselecting an already selected node doesn’t fire the ops/tree_select POST request, which causes cy.wait to fail.
Selecting "Server: EVM" node, api is fired:

Reselecting the node, api is not fired:

But in other scenarios, it does fire.
Selecting "Environment/Dev" node, api is fired:

Reselecting the node, api is fired again:

The solution is to improve interceptApi so that it conditionally waits only when a request is actually initiated.
Cypress doesn’t expose a global event for intercepts, I’ll need to explore other common events as a workaround.