NYULibraries / primo-explore-views

A consolidated monorepo of NYU Consortium views

MIT License

2 stars 1 forks source link

See also CircleCI workflows for July 20-21, 2022 for errors and the commits associated with them. Note alternating success and failure for exact same code under test.

Current theory working theory: either libraryh3lp backend outages or application + network race conditions are causing the .libraryh3lp element to be absent from the DOM. This causes this test to fail in Cypress 10 (which we just upgraded to):

    it('does not have a visible libraryh3lp element', () => {
      cy.get('.libraryh3lp')
        .should('not.be.visible')
    })

This test expects an element with class "libraryh3lp" to always be present but initially hidden. There was a bug in versions of Cypress earlier than 6 to cause cy.get(<ELEMENT>).should('not.be.visible') to false pass if did not exist. Thus regardless of whether the div.libraryh3lp element was present and hidden or not present at all, our test would return true. As of Cypress 6, this test will now fail due to .libraryh3lp being completely absent, not just present and not visible.

These contradictory error messages in the CircleCI output illustrate the point:

AssertionError: Timed out retrying after 12000ms: Expected to find element: `.libraryh3lp`, but never found it.
    at Context.eval (http://web-test:8004/__cypress/tests?p=cypress/e2e/NYU/primo-explore-libraryh3lp-widget.cy.js:108:30)

...and...

AssertionError: Timed out retrying after 12000ms: Expected <div.libraryh3lp> not to exist in the DOM, but it was continuously found.
    at Context.eval (http://web-test:8004/__cypress/tests?p=cypress/e2e/NYU/primo-explore-libraryh3lp-widget.cy.js:108:30)

(Note that the above messages usually were associated with different test code. For a brief period, should(not.be.visible) was replaced with should(not.exist).

We need to eliminate this test flake.

This is very likely a manifestation of the failure mode causing the test failure:

libraryh3lp-error

It tends to clear itself. Haven't managed to get the HTML for it yet. EDIT: Got it:

<prm-silent-login-after parent-ctrl="$ctrl">
  <button class="button chat-tab ss-chat js-toggle-chat" ng-class="$ctrl.klasses" ng-init="showChatWidget = false" ng-click="showChatWidget = !showChatWidget">
    <prm-icon style="z-index:1" icon-type="svg" svg-icon-set="communication" icon-definition="ic_chat_24px">
      <!---->
      <md-icon ng-if="::($ctrl.iconDefinition &amp;&amp; !$ctrl.isCustom &amp;&amp; !$ctrl.isEmailMode())" md-svg-icon="communication:ic_chat_24px" role="presentation" class="md-primoExplore-theme">
        <svg width="100%" height="100%" viewBox="0 0 24 24" id="ic_chat_24px_cache0" y="192" xmlns="http://www.w3.org/2000/svg" fit="" preserveAspectRatio="xMidYMid meet" focusable="false">
          <path d="M20 2H4c-1.1 0-1.99.9-1.99 2L2 22l4-4h14c1.1 0 2-.9 2-2V4c0-1.1-.9-2-2-2zM6 9h12v2H6V9zm8 5H6v-2h8v2zm4-6H6V6h12v2z"></path>
        </svg>
      </md-icon>
      <!---->
      <!---->
      <prm-icon-after parent-ctrl="$ctrl"></prm-icon-after>
    </prm-icon> Chat with us
  </button>
  <div class="chat-frame-wrap" ng-class="$ctrl.klasses" ng-show="showChatWidget" aria-hidden="false" style="">
    <button class="chat-close ss-icon js-toggle-chat" title="Close chat window" ng-click="showChatWidget = !showChatWidget" aria-label="Close chat window">×</button>
    <!---->
    <!---->Chat is offline.
    <!---->
  </div>
</prm-silent-login-after>

Here's what the widget normally looks like (prettified):

Here's the HTML for it:

<prm-silent-login-after parent-ctrl="$ctrl">
  <button class="button chat-tab ss-chat js-toggle-chat" ng-class="$ctrl.klasses" ng-init="showChatWidget = false" ng-click="showChatWidget = !showChatWidget">
    <prm-icon style="z-index:1" icon-type="svg" svg-icon-set="communication" icon-definition="ic_chat_24px">
      <!---->
      <md-icon ng-if="::($ctrl.iconDefinition &amp;&amp; !$ctrl.isCustom &amp;&amp; !$ctrl.isEmailMode())" md-svg-icon="communication:ic_chat_24px" role="presentation" class="md-primoExplore-theme">
        <svg width="100%" height="100%" viewBox="0 0 24 24" id="ic_chat_24px_cache11" y="192" xmlns="http://www.w3.org/2000/svg" fit="" preserveAspectRatio="xMidYMid meet" focusable="false">
          <path d="M20 2H4c-1.1 0-1.99.9-1.99 2L2 22l4-4h14c1.1 0 2-.9 2-2V4c0-1.1-.9-2-2-2zM6 9h12v2H6V9zm8 5H6v-2h8v2zm4-6H6V6h12v2z"></path>
        </svg>
      </md-icon>
      <!---->
      <!---->
      <prm-icon-after parent-ctrl="$ctrl"></prm-icon-after>
    </prm-icon> Chat with us
  </button>
  <div class="chat-frame-wrap" ng-class="$ctrl.klasses" ng-show="showChatWidget" aria-hidden="false" style="">
    <button class="chat-close ss-icon js-toggle-chat" title="Close chat window" ng-click="showChatWidget = !showChatWidget" aria-label="Close chat window">×</button>
    <!---->
    <!---->
    <div class="libraryh3lp" style="" data-lh3-jid="nyu-aal-chat@chat.libraryh3lp.com">
      <iframe title="Ask a Librarian chat" src="https://libraryh3lp.com/chat/nyu-aal-chat@chat.libraryh3lp.com?skin=23106" frameborder="1" style="width: 186px; height: 250px; border: 2px solid rgb(192, 192, 192);"></iframe>
    </div>
    <!---->
  </div>
</prm-silent-login-after>

Here's what the widget looks like when it's closed -- and note that it looks like this even when chat is offline (which the user discovers after clicking the button):

Here's the HTML for the closed widget (prettified):

<prm-silent-login-after parent-ctrl="$ctrl">
   <button class="button chat-tab ss-chat js-toggle-chat" ng-class="$ctrl.klasses" ng-init="showChatWidget = false" ng-click="showChatWidget = !showChatWidget">
      <prm-icon style="z-index:1" icon-type="svg" svg-icon-set="communication" icon-definition="ic_chat_24px">
         <!---->
         <md-icon ng-if="::($ctrl.iconDefinition &amp;&amp; !$ctrl.isCustom &amp;&amp; !$ctrl.isEmailMode())" md-svg-icon="communication:ic_chat_24px" role="presentation" class="md-primoExplore-theme">
            <svg width="100%" height="100%" viewBox="0 0 24 24" id="ic_chat_24px_cache11" y="192" xmlns="http://www.w3.org/2000/svg" fit="" preserveAspectRatio="xMidYMid meet" focusable="false">
               <path d="M20 2H4c-1.1 0-1.99.9-1.99 2L2 22l4-4h14c1.1 0 2-.9 2-2V4c0-1.1-.9-2-2-2zM6 9h12v2H6V9zm8 5H6v-2h8v2zm4-6H6V6h12v2z"></path>
            </svg>
         </md-icon>
         <!----><!---->
         <prm-icon-after parent-ctrl="$ctrl"></prm-icon-after>
      </prm-icon>
      Chat with us
   </button>
   <div class="chat-frame-wrap ng-hide" ng-class="$ctrl.klasses" ng-show="showChatWidget" aria-hidden="true" style="">
      <button class="chat-close ss-icon js-toggle-chat" title="Close chat window" ng-click="showChatWidget = !showChatWidget" aria-label="Close chat window">×</button>
      <!---->
      <!---->
      <div class="libraryh3lp" style="" data-lh3-jid="nyu-aal-chat@chat.libraryh3lp.com"><iframe title="Ask a Librarian chat" src="https://libraryh3lp.com/chat/nyu-aal-chat@chat.libraryh3lp.com?skin=23106" frameborder="1" style="width: 186px; height: 250px; border: 2px solid rgb(192, 192, 192);"></iframe></div>
      <!---->
   </div>
</prm-silent-login-after>

References

libraryh3lp
- chat widget
- Want to chat with patrons browsing your LibGuides?
- Provide instant help for guests browsing your website
- Use live chat to help guests on-the-go browsing your mobile website
- Python SDK reference: LibraryH3lp API

Branch: https://github.com/NYULibraries/primo-explore-views/tree/384_libraryh3lp-e2e-test-flake

'does not have a visible close button' test is broken

button.close-chat should be button.chat-close:

A later test does use the correct selector. This is not related to the test flakiness being addressed in this ticket. It is a broken test that always returns a false pass. I'll fix it along with the flaky test (which is also a test of initial load state).

EDIT: Note that the above was from master branch. In development it was recently changed to this:

it('does not have a visible close button', () => { cy.get('button.close-chat') .should('not.exist') })

...as part of the upgrade of Cypress to 10. It passes because button.chose-chat will never exist regardless of the state of the widget. I'll change it back to not.be.visible -- but note that there is a chance this will fail, because even though the close button is not visible to the human on initial load, the display property is computed value "block":

There are many other ways to make an element not visible other than using the display property though, and Cypress probably will know all the methods and check them.

Preliminary research done:

Read the libraryh3lp "docs" for the chat widget (I actually could find the docs, but I read the marketing pages)
Read the HTML/CSS/JS for our customization NPM package and the monorepo code for injecting the widget. Unfortunately the source map does not appear to be working so couldn't step through it in Developer Tools debugger.
Read the e2e tests
Scanned hard to read optimized JS of the libraryh3lp script being injected: https://libraryh3lp.com/js/libraryh3lp.js?7516

The near-term goal is to get all green for development CircleCI tests so that Vlad can merge into master.

I believe the intent of the flaky test is to establish that widget initially loads in a closed state, with no chat (and the test mentioned previously checks that the close button is not displayed). To fulfill this intent we don't need to concern ourselves with whether the libraryh3lp service is working or not, because the widget in a closed state displays exactly the same in both online and offline (error) state. The solution is to use a different element for testing. The <code>div.libraryh3lp</code> element currently being used is not reliable because when apparently when connection to the backend service breaks this element is never created. The wrapper element <code>div.chat-frame-wrap</code>, however, should always exist (whether visible or not). here the widget is closed:</p> <img width="1917" alt="image" src="https://user-images.githubusercontent.com/5719179/180328506-0b7e6af3-f6cf-42f8-b4e9-6df1dcb4b89d.png"> <p>...and open:</p> <img width="1920" alt="image" src="https://user-images.githubusercontent.com/5719179/180328539-c5bfd24b-08a3-4c40-b082-040bf9ab724c.png"> <p>Note the change in <code>display</code> property from "none" to "block". This element is already being used in the widget-open tests -- <a href="https://github.com/NYULibraries/primo-explore-views/blob/master/primo-explore-e2e-cypress/cypress/integration/NYU/primo-explore-libraryh3lp-widget.spec.js#L28-L31">https://github.com/NYULibraries/primo-explore-views/blob/master/primo-explore-e2e-cypress/cypress/integration/NYU/primo-explore-libraryh3lp-widget.spec.js#L28-L31</a>:</p> <pre><code class="language-javascript"> it('it opens a visible libraryh3lp iframe', () => { cy.get('div.chat-frame-wrap') .should('be.visible') })</code></pre> <p>...so in fact it would be nicely symmetrical to use the same element to test open and close state.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/da70"><img src="https://avatars.githubusercontent.com/u/5719179?v=4" />da70</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>Pushed up the change, and tests pass: <a href="https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1121/workflows/e11e55f1-740b-4cf7-97a7-f6ac9c056554">https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1121/workflows/e11e55f1-740b-4cf7-97a7-f6ac9c056554</a>.</p> <p>Running the CircleCI build 4x times. Meant to re-run it once but kept clicking it because couldn't tell if it was doing anything. We need to test for CircleCI flakiness anyway.</p> <p>Also need to manually red light the tests (per red-green-refactor TDD practice). The VirtualBox on the laptop I'm on has a problem, and so I can't start up the dockerized local Primo dev environment, and I don't want to waste CircleCI jobs for manual red lights.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/da70"><img src="https://avatars.githubusercontent.com/u/5719179?v=4" />da70</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>I suspect that the "Chat is offline" state is not the result of a technical failure. Yesterday Iris asked reference-services-group@nyu.edu staff if there had been any reports of issues with AAL chat or email -- see email thread subject "libraryh3lp Issues?" started by Iris on 2022-07-21 at 1:26PM. Staffers reported that between 1-2pm their "availability" (quotes theirs) was probably cutting in/out for 1-2 minutes. (It may have been caused by bad storm conditions -- one staffer said their AAL Chat connectivity was flashing purple and green right when a big storm hit). I took the "Chat is offline" screenshot during this time period, and the offline status appeared to resolve itself within minutes. Iris wondered if the widget offline status happens when the live AAL person isn't able to log in (due to a storm, for example). She said she'd check with Meg. Vlad and I both noticed that chat was offline after New York hours were over. I will check again tonight, and if the widget is in offline state I will copy the HTML to this ticket.</p> <p>This being the case, determining whether the chat is offline or not is not within scope of these tests. Presumably the widget and the customizations are still working properly when they are reporting that the service is offline, so the tests should pass. Also, the libraryh3lp widget is inside an <iframe> which is not inspectable due to Cypress technical limitations, so it's not really possible to write tests that check widget state even if we felt that was in scope. There is the possibility that the "Chat is offline" message appears outside of an <iframe>. I will check just because it would be good to know.</p> <p>The last six CircleCI workflow tests succeeded. I also disabled the customization on my local and confirmed that all tests failed.</p> <p>Merged PR into <code>development</code>: <a href="https://github.com/NYULibraries/primo-explore-views/pull/385">https://github.com/NYULibraries/primo-explore-views/pull/385</a></p> <p>Vlad will merge into <code>master</code> with some other changes when he deploys some work to prod.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/da70"><img src="https://avatars.githubusercontent.com/u/5719179?v=4" />da70</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>Taken just now, 5:12pm:</p> <img width="2557" alt="image" src="https://user-images.githubusercontent.com/5719179/180569398-f006c1a7-2208-47be-a7b2-e9d92a524223.png"> <p>Got the HTML and will put it in Description.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/da70"><img src="https://avatars.githubusercontent.com/u/5719179?v=4" />da70</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>The "Chat is offline." status message is not inside an <iframe>, and in fact no <iframe> is generated at all when chat is offline. We should have the ability to detect this status using Cypress, if for whatever reason we need to.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/da70"><img src="https://avatars.githubusercontent.com/u/5719179?v=4" />da70</a> commented <strong> 2 years ago</strong> </div> <div class="markdown-body"> <p>From Iris, just now:</p> <ul> <li>AAL stands for Ask A Librarian</li> <li>Service is more or less 24hrs, with NYUAD and NYSH helping with coverage outside of New York business hours. Hours do fluctuate though, based on staffing availability, and Iris believes that staff do have some sort of handoff when they change their shifts (which is multiple times per day). Summer is usually more lightly staffed, but during peak semester periods the reference group does try for 24-hour coverage. The "Ask a Librarian Virtual Reference" section of <a href="https://guides.nyu.edu/library-hours#s-lg-box-24031980">NYU Libraries Hours</a> for the exact schedule. Here's what's there currently:</li> </ul> <img width="894" alt="image" src="https://user-images.githubusercontent.com/5719179/180571974-bc0f1a7a-65c9-4571-b7b3-8ef1528fc6c1.png"> <p>Looks like it's a dynamically updated schedule -- Friday is highlighted, and status "Chat is offline" is displayed.</p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>

NYULibraries / primo-explore-views

libraryh3lp e2e test flake caused by sporadic absence of .libraryh3lp element being tested #384

References

'does not have a visible close button' test is broken