Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
GrayedFox opened this issue Sep 16, 2024 · 5 comments
Open

Comments

@GrayedFox
Copy link

GrayedFox commented Sep 16, 2024

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 👍🏽

@GrayedFox GrayedFox changed the title Ability to further extend expect-expect to check to allow for class or other object/lib methods Ability to further extend expect-expect to match any function/method name Sep 16, 2024
@mskelton
Copy link
Member

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
Copy link

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
Copy link
Member

mskelton commented Nov 9, 2024

@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.

@slhsxcmy
Copy link

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.

Hi @mskelton, I'm wondering why supporting regex is discouraged. In my use case, I would like configure the lint rule to consider any helper functions that starts with assert to be an assertion.

@mskelton
Copy link
Member

@slhsxcmy Hmm, you make a fair point, perhaps regex is the way to go. I would say then we should do the following:

  1. Update the logic to parse member expressions into agent.seesAnything.
  2. For the matching, support regex so you can match assert.*, agent\.sees.*, etc. The world is your oyster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants