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

Gauge freezes when running async methods #199

Closed
jensakejohansson opened this issue Jan 23, 2024 · 12 comments · Fixed by #201
Closed

Gauge freezes when running async methods #199

jensakejohansson opened this issue Jan 23, 2024 · 12 comments · Fixed by #201

Comments

@jensakejohansson
Copy link

jensakejohansson commented Jan 23, 2024

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

sriv commented Jan 23, 2024

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

@jensakejohansson
Copy link
Author

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

sriv commented Jan 24, 2024

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.

@sriv sriv closed this as completed Jan 24, 2024
@sriv sriv reopened this Jan 24, 2024
@jensakejohansson
Copy link
Author

jensakejohansson commented Jan 24, 2024

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

sriv commented Jan 24, 2024

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

jensakejohansson commented Jan 24, 2024

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

sriv commented Jan 24, 2024

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

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

sriv added a commit that referenced this issue Jan 29, 2024
@sriv
Copy link
Member

sriv commented Jan 30, 2024

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

jensakejohansson commented Jan 31, 2024

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 added a commit that referenced this issue Feb 6, 2024
the entire method execution chain is now async

Signed-off-by: sriv <[email protected]>
sriv added a commit that referenced this issue Feb 6, 2024
sriv added a commit that referenced this issue Feb 6, 2024
the entire method execution chain is now async

Signed-off-by: sriv <[email protected]>
sriv added a commit that referenced this issue Feb 6, 2024
sriv added a commit that referenced this issue Feb 6, 2024
the entire method execution chain is now async

Signed-off-by: sriv <[email protected]>
@sriv sriv closed this as completed in #201 Feb 6, 2024
sriv added a commit that referenced this issue Feb 6, 2024
* check if method is async and await results, #199

Signed-off-by: sriv <[email protected]>

* Add .NET 8 support (#200)

* Add .NET 8 support

Signed-off-by: Martin Pekurny <[email protected]>

* Update the init command to create a newer csproj file and removed the matrix for different dotnet versions in the build.yml

Signed-off-by: Martin Pekurny <[email protected]>

* Limiting the number of nodes to use for the feature tests

Signed-off-by: Martin Pekurny <[email protected]>

---------

Signed-off-by: Martin Pekurny <[email protected]>
Signed-off-by: sriv <[email protected]>

* add check to guard against async void impl

step implementations with async void are treated as not implemented

Signed-off-by: sriv <[email protected]>

* bump version 0.6.0

Signed-off-by: sriv <[email protected]>

* fix nunit assert syntax

Signed-off-by: sriv <[email protected]>

* make MethodExecutor.Execute handle async, #199

the entire method execution chain is now async

Signed-off-by: sriv <[email protected]>

* preserve arguments check when executing hook

Signed-off-by: sriv <[email protected]>

---------

Signed-off-by: sriv <[email protected]>
Signed-off-by: Martin Pekurny <[email protected]>
Co-authored-by: mpekurny <[email protected]>
sriv added a commit that referenced this issue Feb 13, 2024
@sriv
Copy link
Member

sriv commented Feb 13, 2024

Reopened due to getgauge/gauge#204

@mpekurny
Copy link
Contributor

@jensakejohansson have you tried out the new async update? While it seems that there might be a new issue described here, #241, can we close this issue?

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

Successfully merging a pull request may close this issue.

4 participants