playwright-dotnet icon indicating copy to clipboard operation
playwright-dotnet copied to clipboard

[Feature] Add Expect.Poll

Open mxschmitt opened this issue 3 years ago • 5 comments

See here: https://playwright.dev/docs/test-assertions#polling

mxschmitt avatar Sep 14 '22 08:09 mxschmitt

Would the suggestion below be a good starting point? The "problem" (as my simple mind sees the solution) is that you don't need to inherit from AssertionsBase for anything (as you will just retry calling the function parameter - but this prevents knowing the default timeout set in AssertionsBase. Unless it's okay to set a separate default timeout for polling.

FYI my current solution is further below. If a similar thing is good enough (but including backoff intervals and a toBe method) I could give this a shot.

public static class Assertions
{
    // new line, pseudocode-ish:
    public static IPollAssertions Expect<T>(Func<Task<T>> function) => new PollAssertions(function);

    public static ILocatorAssertions Expect(ILocator locator) => new LocatorAssertions(locator, false);
    public static IPageAssertions Expect(IPage page) => new PageAssertions(page, false);
    public static IAPIResponseAssertions Expect(IAPIResponse response) => new APIResponseAssertions(response, false);
}

My current solution:

public static async Task ShouldBeEqual(this Assert assert, string expected, Func<Task<string?>> function)
{
    // Retry method is a do-while loop with Thread.Sleep for the polling intervals, throwing an exception on failure
    await retrier.Retry(async () => Assert.AreEqual(expected, await function()));
}

Exoow avatar Jan 16 '24 13:01 Exoow

Would the suggestion below be a good starting point?

The "problem" (as my simple mind sees the solution) is that you don't need to inherit from AssertionsBase for anything (as you will just retry calling the function parameter - but this prevents knowing the default timeout set in AssertionsBase. Unless it's okay to set a separate default timeout for polling.

FYI my current solution is further below. If a similar thing is good enough (but including backoff intervals and a toBe method) I could give this a shot.


public static class Assertions

{

    // new line, pseudocode-ish:

    public static IPollAssertions Expect<T>(Func<Task<T>> function) => new PollAssertions(function);



    public static ILocatorAssertions Expect(ILocator locator) => new LocatorAssertions(locator, false);

    public static IPageAssertions Expect(IPage page) => new PageAssertions(page, false);

    public static IAPIResponseAssertions Expect(IAPIResponse response) => new APIResponseAssertions(response, false);

}

My current solution:


public static async Task ShouldBeEqual(this Assert assert, string expected, Func<Task<string?>> function)

{

    // Retry method is a do-while loop with Thread.Sleep for the polling intervals, throwing an exception on failure

    await retrier.Retry(async () => Assert.AreEqual(expected, await function()));

}

I would recommend Task.Delay instead, otherwise it looks reasonable!

mxschmitt avatar Jan 16 '24 13:01 mxschmitt

@mxschmitt could you have a look at the linked commit please? Is this usable? Also, do I need to do anything with the "API generation" tool mentioned in the Contributing? I manually added interfaces to the Generated folder in the project.

Exoow avatar Jan 24 '24 15:01 Exoow

Looks great! But more like Expect.ToPass rather than Poll. I think we could reshape it a bit, so it matches with the JavaScript API for ToPass and then close the Expect.Poll feature request:

        await Expect(async () =>
        {
            var foo = await Page.EvaluateAsync<string>("() => document.title");
            Assert.AreEqual("Hello world", foo);
        }).ToPassAsync(new () {
            Intervals = new[] { 1_000, 2_000, 10_000 },
            Timeout = 60_000,
        });

Something like this I was envisioning.

If we want to implement expect.Poll, it should look like this:

await Expect.Poll(async () => {
  var response = await Page.APIRequest.GetAsync("https://api.example.com");
  return response.Status;
}, new() {
  // Custom error message, optional.
  message = "make sure API eventually succeeds", // custom error message
  // Poll for 10 seconds; defaults to 5 seconds. Pass 0 to disable timeout.
  timeout: 10000,
}).toBeAsync(200);

Which is much more effort to implement, since we don't have the "default expect matchers like toBe" in .NET compared to Node.js where we get them from the expect Library. In .NET we have Assert for that. So this seems out of scope and ToPass like you did seems like a great feature!

mxschmitt avatar Jan 30 '24 08:01 mxschmitt

@mxschmitt Okay, I understand and will adapt. I'll also update the logic so it ignores any exceptions until success or timeout. Thanks for the feedback so far!

Exoow avatar Jan 30 '24 09:01 Exoow