getgauge / gauge-dotnet

C# runner for gauge + DotNet standard 6.0-8.0
Apache License 2.0
27 stars 21 forks source link

Gauge freezes when running async methods #199

Closed jensakejohansson closed 1 month ago

jensakejohansson commented 10 months ago

I have problems with Gauge/C# that the execution halts whenever async-methods are invoked and the result is waited for. This makes it difficult to use, for example, Playwright with Gauge since it only provides an async API for dotnet. Example:

[Step("Start Chrome")]
public void SetLanguageVowels()
{
    var playwright = Playwright.CreateAsync().Result;
    var browser = playwright.Chromium.LaunchAsync(new BrowserTypeLaunchOptions { Headless = false }).Result;
    var page = browser.NewPageAsync().Result;
    page.GotoAsync("http://www.google.com/");
}

If you execute this step it will stop at the first line indefinitely and never return. To be clear, it's not Playwright specific, any async method-call that you wait for like this seems to result in some sort of deadlock.

A work-around is to run the code in a seperate thread, which works:

[Step("Start Chrome in thread")]
public async void StartChromeInThread()
{
    var t = Task.Run(() =>
    {
        var playwright = Playwright.CreateAsync().Result;
        var browser = playwright.Chromium.LaunchAsync(new BrowserTypeLaunchOptions { Headless = false }).Result;
        var page = browser.NewPageAsync().Result;
        page.GotoAsync("http://www.google.com/");
    });
   t.Wait();
}

However, this does not scale well in our test framework and alot of complexity arises without providing anything but problems...

@sriv Any idea why the first example freezes Gauge? This is unfortunately a quite serious limitation in Gauge/C#.

sriv commented 10 months ago

Not at the computer but what happens when you make the method async? If my memory is right, gauge-dotnet honours async implementations

jensakejohansson commented 10 months ago

Execution just halts. I don't know the details of how the plugin works but it freezes trying to execute the task.

Thanks for replying, we really like Gauge and we use it in several projects and we hope we can continue to do so even if we understand that the development/support is very limited...

If you have time I have created a minimal step/spec impl that reproduces the problem, no dependencies needed:

https://github.com/jensakejohansson/gauge-async

sriv commented 10 months ago

I guess I wasn't clear. I was suggesting something like this (from your sample project):

        /// <summary>
        /// This step will freeze.
        /// </summary>
        [Step("Test async")]
        public async void TestAsync()
        {
            int result = await DoSomethingAsync();
            Console.WriteLine("Completed");
        }

The TestAsync method can be made an async and gauge will honour it. I can see it run fine on my machine.

PS - apologies for closing the issue accidentally.

jensakejohansson commented 10 months ago

If I run you step above you are correct that it does no longer freeze, but maybe I should rephrase my problem: The output from your example above is be:

# Specification Heading
  ## Test Async
Doing something...

Successfully generated html-report to => c:\Users\JensJ\Projects\gauge-async\reports\html-report\index.html

Specifications: 1 executed  1 passed    0 failed    0 skipped
Scenarios:  1 executed  1 passed    0 failed    0 skipped

The result from DoSomethingAsync() is not waited for and the following prints are not executed, the test execution just continues (in this case completes). I need to be able to wait for DoSomethingAsync to complete and then run the next line of code, just as if the code was synchronous.

That is, how do I wait for the result of DoSomethingAsync() before continuing execution of the test (step)? I expect/need the following output from the example:

# Specification Heading
  ## Test Async
Doing something...
Done something.
Completed
sriv commented 10 months ago

Strange, I see this:

$ gauge run
MSBuild version 17.7.4+3ebbd7c49 for .NET
# Specification Heading
  ## Test Async Doing something...
Done something.
Completed
 ✔Doing something...
 ✔

Successfully generated html-report to => /tmp/gauge-async/reports/html-report/index.html

Specifications: 1 executed  1 passed    0 failed    0 skipped
Scenarios:  1 executed  1 passed    0 failed    0 skipped

Total time taken: 2.124s
Updates are available. Run `gauge update -c` for more info.

Are you sure you are awaiting DoSomething? i.e.

            int result = await DoSomethingAsync();

I will try to send you a PR with my changes if that helps

jensakejohansson commented 10 months ago

Sorry for the confusion. I have commented out the first step, that's why the output differ (see below). Your output as well indicates that the 2nd step does not complete, you don't have the output "Done something." & Completed" for that step.

## Test Async
// Works...
//* Test
// Freezes for ever...
* Test async

Yes, I await. Step:

[Step("Test async")]
public async void TestAsync()
{
    int result = await DoSomethingAsync();
    Console.WriteLine("Completed");
}
sriv commented 10 months ago

Thanks for the clarification. I can see what you mean. Will try to release a fix, I have a hunch on what's missing

jensakejohansson commented 10 months ago

If find the problem and you're going to make new release, feel free to add .NET 8 support as well :)

sriv commented 10 months ago

I've raised a PR with a potential fix for this. But then I also remembered that async void is not a good thing

i.e this is not good:

[Step("Test async")]
public async void TestAsync()
{
    int result = await DoSomethingAsync();
    Console.WriteLine("Completed");
}

and instead it should be this:

[Step("Test async")]
public async Task TestAsync()
{
    int result = await DoSomethingAsync();
    Console.WriteLine("Completed");
}

I am trying to see how to guard against this, should gauge-dotnet break execution? Any thoughts?

jensakejohansson commented 10 months ago

Hi!

So async methods would need to return a Task to work as expected and otherwise we want to halt exectuion? Fair enough. From a user perspective I think it would be fine if Gauge skips the scenario if an async step-method does not return as Task in a similar way as it does when a step implementation is missing: [ValidationError] c:\some_path\example.spec:7 Step implementation not found => 'Test async'

But with another descriptive error message, e.g. "Async step implementations must have return type Task".

Keep me updated about your progress. We started looking at this last week, but didn't solve it so we are eager to see a proposed solution.

sriv commented 9 months ago

Reopened due to getgauge/gauge#204

mpekurny commented 1 month ago

@jensakejohansson have you tried out the new async update? While it seems that there might be a new issue described here, https://github.com/getgauge/gauge-dotnet/issues/241, can we close this issue?