From 8016a220f074d5157e553f2b6444d47594c4f41b Mon Sep 17 00:00:00 2001
From: Max Okorokov <max@okorokov.com>
Date: Thu, 9 Jan 2020 15:26:56 +0100
Subject: [PATCH] test: fix more flaky modal keyboard tests (#3538)

---
 .../modal/autoclose/modal-autoclose.e2e-spec.ts  |  2 +-
 .../src/app/modal/focus/modal-focus.e2e-spec.ts  |  5 +++--
 .../modal-stack-confirmation.component.html      |  4 ++--
 .../modal-stack-confirmation.e2e-spec.ts         |  8 ++++----
 .../modal-stack-confirmation.po.ts               |  6 +++---
 .../app/modal/stack/modal-stack.component.html   |  4 ++--
 .../src/app/modal/stack/modal-stack.e2e-spec.ts  |  8 +++++---
 e2e-app/src/app/modal/stack/modal-stack.po.ts    | 16 +++++++---------
 8 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/e2e-app/src/app/modal/autoclose/modal-autoclose.e2e-spec.ts b/e2e-app/src/app/modal/autoclose/modal-autoclose.e2e-spec.ts
index 634d35480d..c6442b2c9e 100644
--- a/e2e-app/src/app/modal/autoclose/modal-autoclose.e2e-spec.ts
+++ b/e2e-app/src/app/modal/autoclose/modal-autoclose.e2e-spec.ts
@@ -28,7 +28,7 @@ describe('Modal', () => {
 
     // close
     await sendKey(Key.ESCAPE);
-    expect(await page.getModal().isPresent()).toBeFalsy('The modal should be closed on ESC');
+    await expectNoOpenModals('The modal should be closed on ESC');
     expect(await page.getDismissReason()).toBe('Escape', `Modal should have been dismissed with 'Escape' reason`);
   });
 
diff --git a/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts b/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts
index 4356255daf..7d86a31854 100644
--- a/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts
+++ b/e2e-app/src/app/modal/focus/modal-focus.e2e-spec.ts
@@ -19,7 +19,7 @@ describe('Modal', () => {
 
     // close
     await sendKey(Key.ESCAPE);
-    expect(await page.getModal().isPresent()).toBeFalsy('The modal should be closed on ESC');
+    await expectNoOpenModals('The modal should be closed on ESC');
 
     // button should be focused
     await expectFocused($('#open-modal-simple'), 'Should focus trigger button after closing');
@@ -30,7 +30,7 @@ describe('Modal', () => {
 
     // close
     await modal.click();
-    expect(await modal.isPresent()).toBeFalsy('The modal should be closed on ESC');
+    expect(await modal.isPresent()).toBeFalsy('The modal should be closed on click');
 
     // button should be focused
     await expectFocused($('#open-modal-simple'), 'Should focus trigger button after closing');
@@ -41,6 +41,7 @@ describe('Modal', () => {
 
     // close
     await sendKey(Key.ESCAPE);
+    await expectNoOpenModals('The modal should be closed on ESC');
 
     // body should be focused
     await expectFocused($('body'), 'Should focus body after closing');
diff --git a/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.component.html b/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.component.html
index e2dbf38608..de9f59f607 100644
--- a/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.component.html
+++ b/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.component.html
@@ -1,7 +1,7 @@
 <h3>Modal closure confirmation test</h3>
 
 <ng-template #modal let-modal>
-  <div class="modal-header">
+  <div id="modal" class="modal-header">
     <h4 class="modal-title">Modal with closure confirmation</h4>
     <button id="close" type="button" class="close" aria-label="Close" (click)="modal.dismiss()">
       <span aria-hidden="true">&times;</span>
@@ -13,7 +13,7 @@ <h4 class="modal-title">Modal with closure confirmation</h4>
 </ng-template>
 
 <ng-template #confirmation let-modal>
-  <div class="modal-header">
+  <div id="stack-modal" class="modal-header">
     <h4 class="modal-title">Are you sure you want to close the modal?</h4>
     <button id="dismiss" type="button" class="close" aria-label="Close" (click)="modal.dismiss()">
       <span aria-hidden="true">&times;</span>
diff --git a/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.e2e-spec.ts b/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.e2e-spec.ts
index 84bfd17e09..720ef3a309 100644
--- a/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.e2e-spec.ts
+++ b/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.e2e-spec.ts
@@ -2,7 +2,7 @@ import {expectNoOpenModals, openUrl, sendKey} from '../../tools.po';
 import {ModalStackConfirmationPage} from './modal-stack-confirmation.po';
 import {browser, Key, protractor} from 'protractor';
 
-describe('Modal stacked with confirmation', () => {
+describe('Modal stack with confirmation', () => {
   let page: ModalStackConfirmationPage;
 
   beforeAll(() => { page = new ModalStackConfirmationPage(); });
@@ -35,17 +35,17 @@ describe('Modal stacked with confirmation', () => {
 
     // close with Escape
     await sendKey(Key.ESCAPE);
-    browser.wait(protractor.ExpectedConditions.presenceOf(page.getDismissalButton()));
+    browser.wait(protractor.ExpectedConditions.presenceOf(page.getStackModal()));
     expect(await page.getOpenModals().count()).toBe(2, 'Confirmation modal should be opened');
 
     // cancel closure with Escape
     await sendKey(Key.ESCAPE);
-    browser.wait(protractor.ExpectedConditions.invisibilityOf(page.getDismissalButton()));
+    browser.wait(protractor.ExpectedConditions.invisibilityOf(page.getStackModal()));
     expect(await page.getOpenModals().count()).toBe(1, 'Confirmation modal should be dismissed');
 
     // close again
     await sendKey(Key.ESCAPE);
-    browser.wait(protractor.ExpectedConditions.presenceOf(page.getDismissalButton()));
+    browser.wait(protractor.ExpectedConditions.presenceOf(page.getStackModal()));
     expect(await page.getOpenModals().count()).toBe(2, 'Confirmation modal should be re-opened');
 
     // close all modals
diff --git a/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.po.ts b/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.po.ts
index 64e71a904e..ab9569c8a1 100644
--- a/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.po.ts
+++ b/e2e-app/src/app/modal/stack-confirmation/modal-stack-confirmation.po.ts
@@ -5,6 +5,8 @@ export class ModalStackConfirmationPage {
 
   getModal(index) { return this.getOpenModals().get(index); }
 
+  getStackModal() { return $('#stack-modal'); }
+
   getModalButton() { return $('#open-modal'); }
 
   getModalCloseButton() { return $('#close'); }
@@ -15,8 +17,6 @@ export class ModalStackConfirmationPage {
 
   async openModal() {
     await this.getModalButton().click();
-    const modal = this.getModal(0);
-    expect(await modal.isPresent()).toBeTruthy(`A modal should have been opened`);
-    return modal;
+    expect(await this.getModal(0).isPresent()).toBeTruthy(`A modal should have been opened`);
   }
 }
diff --git a/e2e-app/src/app/modal/stack/modal-stack.component.html b/e2e-app/src/app/modal/stack/modal-stack.component.html
index 59662d9336..884034a100 100644
--- a/e2e-app/src/app/modal/stack/modal-stack.component.html
+++ b/e2e-app/src/app/modal/stack/modal-stack.component.html
@@ -1,7 +1,7 @@
 <h3>Modal nesting tests</h3>
 
 <ng-template #t let-modal>
-  <div class="modal-header">
+  <div id="modal" class="modal-header">
     <h4 class="modal-title">Modal with nested popups</h4>
     <button type="button" class="close" aria-label="Close" (click)="modal.dismiss()">
       <span aria-hidden="true">&times;</span>
@@ -14,7 +14,7 @@ <h4 class="modal-title">Modal with nested popups</h4>
 </ng-template>
 
 <ng-template #t2 let-modal>
-  <div class="modal-header">
+  <div id="stack-modal" class="modal-header">
     <h4 class="modal-title">Stacked modal</h4>
     <button type="button" class="close" aria-label="Close" (click)="modal.dismiss()">
       <span aria-hidden="true">&times;</span>
diff --git a/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts b/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts
index 8f9e32b056..00d6a79f6c 100644
--- a/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts
+++ b/e2e-app/src/app/modal/stack/modal-stack.e2e-spec.ts
@@ -1,10 +1,10 @@
-import {$, Key} from 'protractor';
+import {$, browser, Key, protractor} from 'protractor';
 import {expectFocused, expectNoOpenModals, openUrl, sendKey} from '../../tools.po';
 import {ModalStackPage} from './modal-stack.po';
 
 const bodyClass = () => $('body').getAttribute('class');
 
-describe('Modal stacked', () => {
+describe('Modal stack', () => {
   let page: ModalStackPage;
 
   beforeAll(() => { page = new ModalStackPage(); });
@@ -20,16 +20,18 @@ describe('Modal stacked', () => {
 
     // close the stack modal
     await sendKey(Key.ESCAPE);
+    browser.wait(protractor.ExpectedConditions.invisibilityOf(page.getStackModal()));
     expect(await bodyClass()).toContain('modal-open', `body should have 'modal-open' class`);
 
     // Check that the button is focused again
     await expectFocused(page.getStackModalButton(), 'Button element not focused');
     await sendKey(Key.TAB);
 
-    await expectFocused(page.getCoseIcon(), 'Close icon not focused');
+    await expectFocused(page.getCloseIcon(), 'Close icon not focused');
 
     // close the main modal
     await sendKey(Key.ESCAPE);
+    browser.wait(protractor.ExpectedConditions.invisibilityOf(page.getModal()));
     expect(await bodyClass()).not.toContain('modal-open', `body should have 'modal-open' class`);
   });
 
diff --git a/e2e-app/src/app/modal/stack/modal-stack.po.ts b/e2e-app/src/app/modal/stack/modal-stack.po.ts
index 2227bcd5f2..764a62e0db 100644
--- a/e2e-app/src/app/modal/stack/modal-stack.po.ts
+++ b/e2e-app/src/app/modal/stack/modal-stack.po.ts
@@ -1,25 +1,23 @@
-import {$, $$} from 'protractor';
+import {$} from 'protractor';
 
 export class ModalStackPage {
-  getModal(index) { return $$('ngb-modal-window').get(index); }
+  getModal() { return $('#modal'); }
+
+  getStackModal() { return $('#stack-modal'); }
 
   getModalButton() { return $('#open-modal'); }
 
   getStackModalButton() { return $('#open-inner-modal'); }
 
-  getCoseIcon() { return $('button.close'); }
+  getCloseIcon() { return $('button.close'); }
 
   async openModal() {
     await this.getModalButton().click();
-    const modal = this.getModal(0);
-    expect(await modal.isPresent()).toBeTruthy(`A modal should have been opened`);
-    return modal;
+    expect(await this.getModal().isPresent()).toBeTruthy(`A modal should have been opened`);
   }
 
   async openStackModal() {
     await this.getStackModalButton().click();
-    const modal = this.getModal(1);
-    expect(await modal.isPresent()).toBeTruthy(`A second modal should have been opened`);
-    return modal;
+    expect(await this.getStackModal().isPresent()).toBeTruthy(`A second modal should have been opened`);
   }
 }