Code review & coverage

c

Author: dprevost-LMI

src/util/elementsUtil.ts

@@ -40,6 +40,9 @@ export const isElementOrArrayLike = (obj: unknown): obj is WebdriverIO.ElementAr
     return !!obj && isElement(obj) || isElementArrayLike(obj)
 }
 
+export const isElementOrNotEmptyElementArray = (obj: unknown): obj is WebdriverIO.Element | WdioElements => {
+    return !!obj && isElement(obj) || (isElementArrayLike(obj) && obj.length > 0)
+}
 /**
  * Universaly await element(s) since depending on the type received, it can become complex.
  *

src/util/executeCommand.ts

@@ -27,7 +27,7 @@ export async function executeCommand<T>(
     const { elements, element, other } = await awaitElementOrArray(nonAwaitedElements)
     if (!elements && !element) {
         return {
-            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any -- one day move up the unknown type
             elementOrArray: other as any,
             success: false,
             valueOrArray: undefined,
@@ -45,6 +45,7 @@ export async function executeCommand<T>(
 
     const elementOrArray = element ? element : elements ? elements : undefined
     if (!elementOrArray) {
+        // Should be unreachable due to checks above
         throw new Error('No elements to process in executeCommand')
     }
 

src/util/formatMessage.ts

@@ -1,7 +1,7 @@
 import { printDiffOrStringify, printExpected, printReceived } from 'jest-matcher-utils'
 import { equals } from '../jasmineUtils.js'
 import type { WdioElements } from '../types.js'
-import { isElement, isElementArrayLike, isStrictlyElementArray } from './elementsUtil.js'
+import { isElementOrNotEmptyElementArray, isStrictlyElementArray } from './elementsUtil.js'
 
 export const getSelector = (el: WebdriverIO.Element | WebdriverIO.ElementArray) => {
     let result = typeof el.selector === 'string' ? el.selector : '<fn>'
@@ -55,10 +55,7 @@ export const enhanceError = (
     } = {}): string => {
     const { isNot = false, useNotInLabel = true } = context
 
-    if (isElement(subject) || (isElementArrayLike(subject) && subject.length > 0)) {
-        subject = getSelectors(subject)
-    }
-    subject = subject = typeof subject === 'string' ? subject : JSON.stringify(subject)
+    subject = subject = isElementOrNotEmptyElementArray(subject) ? getSelectors(subject) : toJsonString(subject)
 
     let contain = ''
     if (containing) {
@@ -125,5 +122,18 @@ export const numberError = (options: ExpectWebdriverIO.NumberOptions = {}): stri
         return `<= ${options.lte}`
     }
 
-    return `Incorrect number options provided. Received: ${JSON.stringify(options)}`
+    return `Incorrect number options provided. Received: ${toJsonString(options)}`
+}
+
+const isString = (value: unknown): value is string => typeof value === 'string'
+
+const toJsonString = (value: unknown): string => {
+    if (isString(value)) {
+        return value
+    }
+    try {
+        return JSON.stringify(value)
+    } catch {
+        return String(value)
+    }
 }

src/util/waitUntil.ts

@@ -27,36 +27,37 @@ export const waitUntil = async (
         try {
             result = await condition()
             error = undefined
-            if (typeof result === 'boolean' ? result : result.success) {
+            if (isBoolean(result) ? result : result.success) {
                 break
             }
         } catch (err) {
             error = err
         }
 
         // No need to sleep again if time is already over
-        if (Date.now() - start < wait) {
+        if (canWait(start, wait)) {
             await sleep(interval)
         }
-    } while (Date.now() - start < wait)
+    } while (canWait(start, wait))
 
     if (error) { throw error }
 
-    if (typeof result === 'boolean') {
+    if (isBoolean(result)) {
         return result
     }
 
     const { results } = result
 
     if (results.length === 0) {
-        // To fails when using .not, we need to send true so that Jest inverts it to false
+        // To fails with .not, we need pass=true, so it s inverted later by Jest's expect framework
         return isNot
     }
 
-    // TODO dprevost: Should we consider moving that into a strategy pattern, so that if a matching does not need the same pattern he can change it?
-    const allTrue = () => results.every((result) => result)
-    const allFalse = () => results.every((result) => !result)
-
-    // Needs to return true when all true, but with isNot, must be success=false when all false (failure=true when at least one true)
-    return isNot ? !allFalse() : allTrue()
+    // With isNot to succeed with need pass=false, so it s inverted later by Jest's expect framework
+    return isNot ? !isAllFalse(results) : isAllTrue(results)
 }
+const isBoolean = (value: unknown): value is boolean => typeof value === 'boolean'
+
+const isAllTrue = (results: boolean[]): boolean => results.every((res) => res === true)
+const isAllFalse = (results: boolean[]): boolean => results.every((res) => res === false)
+const canWait = (start: number, wait: number): boolean => (Date.now() - start) < wait

test/matchers.test.ts

@@ -116,7 +116,7 @@ describe('Custom Wdio Matchers Integration Tests', async () => {
         })
 
         // TODO to support one day?
-        test('toHave works with arrayContaining asymmetric matcher', async () => {
+        test.skip('toHave works with arrayContaining asymmetric matcher', async () => {
             await expectLib(el).toHaveText(
                 expectLib.arrayContaining([
                     expectLib.stringContaining('Valid'),

test/matchers/beMatchers.test.ts

@@ -217,7 +217,27 @@ Received: "not ${lastMatcherWords(matcherFn.name)}"`)
                     })
                 })
 
-                test('success with matcherFn and command options', async () => {
+                test('success with matcherFn and custom command options', async () => {
+                    const result = await thisContext.matcherFn(elements, { wait: 4, interval: 99 })
+
+                    for (const element of elements) {
+                        expect(element[elementFnName]).toHaveBeenCalledOnce()
+                    }
+                    expect(waitUntil).toHaveBeenCalledExactlyOnceWith(expect.any(Function), undefined, { wait: 4, interval: 99 })
+                    expect(result.pass).toBe(true)
+                })
+
+                test('success with matcherFn and custom command options - only interval', async () => {
+                    const result = await thisContext.matcherFn(elements, { interval: 99 })
+
+                    for (const element of elements) {
+                        expect(element[elementFnName]).toHaveBeenCalledOnce()
+                    }
+                    expect(waitUntil).toHaveBeenCalledExactlyOnceWith(expect.any(Function), undefined, { wait: 1, interval: 99 })
+                    expect(result.pass).toBe(true)
+                })
+
+                test('success with matcherFn and default command options', async () => {
                     const result = await thisContext.matcherFn(elements)
 
                     for (const element of elements) {

test/softAssertions.test.ts

@@ -28,13 +28,15 @@ describe('Soft Assertions', () => {
             expect(failures[0].error.message).toContain('text')
         })
 
-        // TODO dprevost: fix this, in soft results is undefined even thought the matcher records a failure and returns it
-        it.skip('should support chained assertions with .not', async () => {
+        it('should support chained assertions with .not', async () => {
+            // Setup a test ID for this test
             const softService = SoftAssertService.getInstance()
             softService.setCurrentTest('test-2', 'test name', 'test file')
 
-            await expectWdio.soft(el).not.toHaveText('Actual Text')
+            // This should not throw even though it fails
+            await expectWdio.soft(el).not.toHaveText('Actual Text', { wait: 0 })
 
+            // Verify the failure was recorded
             const failures = expectWdio.getSoftFailures()
             expect(failures.length).toBe(1)
             expect(failures[0].matcherName).toBe('not.toHaveText')

test/util/elementsUtil.test.ts

@@ -1,7 +1,7 @@
 import { vi, test, describe, expect, beforeEach } from 'vitest'
 import { $, $$, } from '@wdio/globals'
 
-import { awaitElementOrArray, awaitElementArray, wrapExpectedWithArray, map, isStrictlyElementArray, isElement, isElementArrayLike, isElementOrArrayLike } from '../../src/util/elementsUtil.js'
+import { awaitElementOrArray, awaitElementArray, wrapExpectedWithArray, map, isStrictlyElementArray, isElement, isElementArrayLike, isElementOrArrayLike, isElementOrNotEmptyElementArray } from '../../src/util/elementsUtil.js'
 import { chainableElementArrayFactory, elementArrayFactory, elementFactory } from '../__mocks__/@wdio/globals.js'
 
 vi.mock('@wdio/globals')
@@ -478,4 +478,27 @@ describe('elementsUtil', () => {
             expect(result).toBe(false)
         })
     })
+
+    describe(isElementOrNotEmptyElementArray, async () => {
+        test.for([
+            await $('element'),
+            await $$('elements'),
+            [elementFactory('element1'), elementFactory('element2')],
+        ])('should return true for Element or non-empty ElementArray/Element[]: %s', async (element) => {
+            const result = isElementOrNotEmptyElementArray(element)
+
+            expect(result).toBe(true)
+        })
+        test.for([
+            [],
+            $$('elements'),
+            $('element'),
+            '1',
+            {},
+        ])('should return false for non-Element or empty array: %s', async (element) => {
+            const result = isElementOrNotEmptyElementArray(element)
+
+            expect(result).toBe(false)
+        })
+    })
 })