playwright-community / eslint-plugin-playwright

ESLint plugin for Playwright
https://www.npmjs.com/package/eslint-plugin-playwright
MIT License
280 stars 39 forks source link

Ability to further extend expect-expect to match any function/method name #316

Open GrayedFox opened 1 month ago

GrayedFox commented 1 month ago

Pretty much an extension of this issue.

We use a sort of scrappy version of the Screenplay pattern, basically just an abstract base class for common testing logic and child classes for each user bucket. Thus many of our assertions live inside a class method:

// example from Agent.ts class
/**
 * Asserts that the status contains {@link content} using {@link Page.getByText}
 */
async seesStatus(status: string) {
    const statusField = this.page.locator('[class*="_status_"]').getByText(status);
    await expect(statusField).toBeVisible();
}

Thus from inside the spec it looks like:

// example spec file
...
  test('enters document and links details', async () => {
      await agent.seesHeading('Documents/Links');
      await agent.seesStatus('Draft');
      await agent.clickDone();
  });

I tried specifying the allowed methods like this but that didn't work and the rule still reported an error:

{
  "playwright/expect-expect": [
    "error",
    {
      "assertFunctionNames": ["agent.seesStatus", "seesStatus"]
    }
  ]
}

I could probably work around this by assigning the class methods like so (untested):

const agent = new FLKAgent({ firstName: 'bob', lastName: 'tester' })
const seesStatus = agent.seesStatus.bind(agent);

What would help our use case and keep our code clean is if we could specify a further property of the eslint rule that, rather than looking for an exact function name or call signature, simply checks that the method name matches a specific character sequence:

{
  "playwright/expect-expect": [
    "error",
    {
      "assertFunctionNames": ["myAssert"],
      "assertFunctionNameMatches": [
        { "startsWith": "sees" },
        { "contains": "checks" },
        { "endsWith": "tested" }
      ]
    }
  ]
}

This rule would enable functions in any form and in any call/execution context to work so long as the name and location logic match, the advantage being that it is agnostic about where the function/method comes from. For example the above config would allow for:

test('testing', async () => {
  await agent.seesAnything(); // class method
  await ThirdPartyLib.property.customCheckCanFly(); // case insensitive match, 3rd party lib
  await (async pieIsTested() { ... })(); // async named IIFE 
})

Shorthand could default match logic to contains:

{
  "playwright/expect-expect": [
    "error",
    {
      "assertFunctionNames": ["myAssert"],
      "assertFunctionNameMatches": ["sees"]
    }
  ]
}

I'm flexible on the schema - if there's a more eslint/PlayWright friendly way to structure the rule options then happy with that, mainly concerned about the logic.

If this sounds like an acceptable suggestion and in line with the plugin I would be happy to open a PR that adds it in due time 👍🏽

mskelton commented 1 month ago

I think what would be best is to simply add support for member expressions (e.g. agent.seesAnything in assertFunctionNames. Let's stick to a single setting, and not support regex right now.

maxime-dupuis commented 1 day ago

Using assertFunctionNames is tedious and leaves us open to, by mistake, listing a function that does not in fact contain a expect statement

Would it be possible to automatically detect when a helper function contains a expect statement?

test('something', async ({ page }) => {
  await helperFunction(page);
});

async function helperFunction(page: Page) {
  await expect(page.locator('div')).toBeVisible(); // All good, there's a expect statement
}
mskelton commented 3 hours ago

@maxime-dupuis I really like that idea a lot. It won't be perfect semantic analysis (e.g. if you import a helper, it won't work), but could be really nice for the majority of single file uses.