webdriverio/visual-testing

Set "!important" to options that adds attributes

BosseKarat opened this issue · 12 comments

BosseKarat commented

Is your feature request related to a problem? Please describe.
I was trying to use 'hideAfterFirstScroll'* today but noticed that I got inconsistent results. I noticed that in some instances setting visibility:hidden isn't enough to hide a speicific element. You also have to add !important you want to get consitent results.

Go to the following page and open up the first modal: https://react.semantic-ui.com/modules/modal/

If you try to remove the modal by setting visibility:hidden it will not work. the only way for me to remove the modal is to also use !important, i.e visibility:hidden!important

https://github.com/wswebcreation/webdriver-image-comparison/blob/main/docs/OPTIONS.md#hideafterfirstscroll-1

Describe the solution you'd like
hideAfterFirstScroll is described as following: This methods will hide 1 or multiple elements by adding the property visibility: hidden to them by providing an array of elements.

In some frameworks (eg. Semantic UI react), setting some elements to visibility:hidden isn't enough. You have to add !important to the attribute or the element will remain visible. Perhaps we would get more consistent results if "!important" is added to the attribute.

Perhaps this could be expanded to every option that adds an attribute: hideElements, removeElements, hideAfterFirstScroll

Describe alternatives you've considered
I can't see any drawbacks in switching from visibility:hidden to visibility:hidden!important and I don't have any other alternatives.

Additional context

BosseKarat commented

I'm a complete noob at this and haven't contributed to any projects at all but if this is the place to make the change I would be happy to raise a PR, hideRemoveElements.ts

bild

wswebcreation commented

Hi @BosseKarat

Thanks for raising this, and yes, you are correct, you need to fix it there. Just make sure you add a space before the !. Feel free to submit a PR and ask questions in the Visual Channel for support

BosseKarat commented

Sorry @wswebcreation, but I can't wrap my head around the toMatchSnapshot assertion which is giving me a hard time due to the changes mentioned above. The function does allow me to pass an argument but I suppose it should be left empty. Do I have to update the snapshot? I tried doing that by passing the --updateSnaphot flag but it only left me with errors.

image

wswebcreation commented

@BosseKarat

I believe you are using the wrong matchers 😅 , see https://webdriver.io/docs/api/expect-webdriverio#visual-snapshot-matchers

We have two types of matchers, the Visual ones and the DOM snapshot ones.

Just curious, because @christian-bromann had a conversation about it, but why did you pick the toMatchSnapshot() one?

wswebcreation commented

🤦 , I checked the wrong thing. Sorry, let me get back to this

wswebcreation commented

When you run test.unit.coverage you will get something like this

 RERUN  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.ts

 ✓ packages/webdriver-image-comparison/src/methods/screenshots.test.ts (9)
 ✓ packages/webdriver-image-comparison/src/helpers/beforeScreenshot.test.ts (2) 1006ms
 ✓ packages/webdriver-image-comparison/src/helpers/afterScreenshot.test.ts (1)
 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts (8)
   ❯ hideRemoveElements (8)
     × should be able to hide elements and put them back again
     × should be able to hide elements and put them back again when an array of hidden elements is provided
     × should be able to remove elements and put them back again
     × should be able to remove elements and put them back again when an array of to be removed elements is provided
     × should be able to find and hide single element based on xpath
     × should be able to find and hide elements based on xpath
     × should be able to find and hide a single element based on a css selector
     × should be able to find and hide elements based on a css selector

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 8 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to hide elements and put them back again
Error: Snapshot `hideRemoveElements > should be able to hide elements and put them back again 3` mismatched

- Expected
+ Received

- "hidden"
+ "hidden !important"

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:31:81


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/8]⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided
Error: Snapshot `hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 5` mismatched

- Expected
+ Received

- "hidden"
+ "hidden !important"

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:73:100


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/8]⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to remove elements and put them back again
Error: Snapshot `hideRemoveElements > should be able to remove elements and put them back again 3` mismatched

- Expected
+ Received

- "none"
+ "none !important"

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:117:78


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/8]⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided
Error: Snapshot `hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 5` mismatched

- Expected
+ Received

- "none"
+ "none !important"

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:159:99


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/8]⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to find and hide single element based on xpath
Error: Snapshot `hideRemoveElements > should be able to find and hide single element based on xpath 3` mismatched

- Expected
+ Received

- "hidden"
+ "hidden !important"

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:203:81


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[5/8]⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to find and hide elements based on xpath
Error: Snapshot `hideRemoveElements > should be able to find and hide elements based on xpath 5` mismatched

- Expected
+ Received

- "hidden"
+ "hidden !important"

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:233:81


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[6/8]⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to find and hide a single element based on a css selector
Error: Snapshot `hideRemoveElements > should be able to find and hide a single element based on a css selector 8` mismatched

- Expected
+ Received

- "hidden"
+ "hidden !important"

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:268:81


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[7/8]⎯

 FAIL  packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts > hideRemoveElements > should be able to find and hide elements based on a css selector
Error: Snapshot `hideRemoveElements > should be able to find and hide elements based on a css selector 2` mismatched

- Expected
+ Received

  NodeList [
    <span
      class="hide"
-     style="visibility: hidden;"
+     style="visibility: hidden !important;"
    >
      Hello
    </span>,
    <span
      class="hide"
-     style="visibility: hidden;"
+     style="visibility: hidden !important;"
    >
      Hello
    </span>,
    <span
      class="hide"
-     style="visibility: hidden;"
+     style="visibility: hidden !important;"
    >
      Hello
    </span>,
    <span
      class="hide"
-     style="visibility: hidden;"
+     style="visibility: hidden !important;"
    >
      Hello
    </span>,
  ]

 ❯ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts:294:78


⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[8/8]⎯

  Snapshots  8 failed
             24 obsolete
             ↳ packages/webdriver-image-comparison/src/clientSideScripts/hideRemoveElements.test.ts
               · hideRemoveElements > should be able to find and hide elements based on xpath 6
               · hideRemoveElements > should be able to find and hide elements based on xpath 7
               · hideRemoveElements > should be able to find and hide elements based on xpath 8
               · hideRemoveElements > should be able to find and hide single element based on xpath 4
               · hideRemoveElements > should be able to hide elements and put them back again 4
               · hideRemoveElements > should be able to hide elements and put them back again 5
               · hideRemoveElements > should be able to hide elements and put them back again 6
               · hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 6
               · hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 7
               · hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 8
               · hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 9
               · hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 10
               · hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 11
               · hideRemoveElements > should be able to hide elements and put them back again when an array of hidden elements is provided 12
               · hideRemoveElements > should be able to remove elements and put them back again 4
               · hideRemoveElements > should be able to remove elements and put them back again 5
               · hideRemoveElements > should be able to remove elements and put them back again 6
               · hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 6
               · hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 7
               · hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 8
               · hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 9
               · hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 10
               · hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 11
               · hideRemoveElements > should be able to remove elements and put them back again when an array of to be removed elements is provided 12

 Test Files  1 failed | 3 passed (4)
      Tests  8 failed | 12 passed (20)
   Start at  10:41:41
   Duration  1.16s


 FAIL  Tests failed. Watching for file changes...
       press u to update snapshot, press h to show help

because this command is running in watch mode you can just press u which will update the snapshots

BosseKarat commented

Thank @wswebcreation - works like a charm. One final question before i pull the trigger; There are a few *.ts.snap files that I don't know what to do with. Should I commit them to my branch or not? I'm thinkingt hey might contain the updated snapshots.

image

Bonus question: One test is complaining about me having a different path. I suppose I shouldn't commit this change because it might break in the pipeline and on your machine.

image

Thank you for your patience - I hope once I get over this first time experience I might be able to help more in the future

wswebcreation commented

Hi @BosseKarat

Awesome, and no problem to ask questions.

There are a few *.ts.snap files that I don't know what to do with. Should I commit them to my branch or not?

Those file contain the new/updated snapshots of the tests. It's an easier way to store data that you need to compare all the time. This external file needs to be committed and will be used on your local machine/in the pipeline to retrieve the snapshot values from when the unit tests are executed

Bonus question: One test is complaining about me having a different path. I suppose I shouldn't commit this change because it might break in the pipeline and on your machine.

hmm, it needs to be the first due to the CI pipeline, but...., this feels like a bad test (not your fault). It should be mocked, not something you should fix. You can now solve it with this

let expectedPath = `${process.cwd()}/something/else.png`;
expectedPath = path.normalize(expectedPath);

expect(foo).toEqual({
  // ....
  path: expectedPath
});
BosseKarat commented

@wswebcreation - I'm getting more errors related to path after syncing my fork 😩 I don't think I can normalize the path because mine is pointing toward c:/. It would solve the back-/forward slash issue but without adjusting the expected value I don't know how to handle this. I didn't have this issue before syncing my branch.

Edit: I guess I could change toBe to toContain but perhaps that's too much of an easy way out

image

wswebcreation commented

ohh, wow, that isn't very pleasant. I already have a PR open to fix an issue which makes that method easier. Let's do the following. Please submit the PR, when my PR is merged we will merge main into your's and we will continue the conversation in the PR.
What do you think?

  • 👍1