Skip to content

Commit

Permalink
fix: video.PathAsync() should not hang after direct launch/close (#2673)
Browse files Browse the repository at this point in the history
Co-authored-by: campersau <[email protected]>
  • Loading branch information
mxschmitt and campersau authored Aug 18, 2023
1 parent c6bcea1 commit 243b87f
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 6 deletions.
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ The resulting code will follow our style guides. This is also enforced in our CI
Tests can either be executed in their entirety:

```bash
dotnet test ./src/Playwright.Tests/Playwright.Tests.csproj --logger:"console;verbosity=detailed"
dotnet test -f net6.0 ./src/Playwright.Tests/Playwright.Tests.csproj --logger:"console;verbosity=detailed"
```

You can also specify a single test to run:

```bash
dotnet test ./src/Playwright.Tests/Playwright.Tests.csproj --logger:"console;verbosity=detailed" --filter Playwright.Tests.TapTests
dotnet test -f net6.0 ./src/Playwright.Tests/Playwright.Tests.csproj --logger:"console;verbosity=detailed" --filter Playwright.Tests.TapTests
```

Additionally, you can use the Test Explorer if you're using Visual Studio.
Expand All @@ -137,7 +137,7 @@ This will re-generate the neccessary files for the new driver version.
```shell
dotnet tool install -g dotnet-reportgenerator-globaltool

dotnet test ./src/Playwright.Tests/Playwright.Tests.csproj --logger:"console;verbosity=detailed" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput="coverage.xml" --filter "Playwright.Tests.Assertions.PageAssertionsTests"
dotnet test -f net6.0 ./src/Playwright.Tests/Playwright.Tests.csproj --logger:"console;verbosity=detailed" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput="coverage.xml" --filter "Playwright.Tests.Assertions.PageAssertionsTests"
reportgenerator -reports:src/Playwright.Tests/coverage.net6.0.xml -targetdir:coverage-report -reporttypes:HTML
open coverage-report/index.html
```
21 changes: 21 additions & 0 deletions src/Playwright.Tests/ScreencastTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,25 @@ public async Task ShouldCaptureStaticPageInPersistentContext()

Assert.IsNotEmpty(new DirectoryInfo(tempDirectory.Path).GetFiles("*.webm"));
}


[PlaywrightTest("screencast.spec.ts", "video.path()/saveAs() does not hang immediately after launchPersistentContext and context.close()")]
[Timeout(30_000)]
public async Task VideoPathSaveAsDoesNotHangImmediatelyAfterLaunchPersistentContextAndContextClose()
{
using var userDirectory = new TempDirectory();
using var tempDirectory = new TempDirectory();

var context = await BrowserType.LaunchPersistentContextAsync(userDirectory.Path, new()
{
RecordVideoDir = tempDirectory.Path
});
var page = context.Pages[0];
await context.CloseAsync();
var exception = await PlaywrightAssert.ThrowsAsync<PlaywrightException>(() => page.Video.PathAsync());
Assert.AreEqual("Page did not produce any video frames.", exception.Message);
exception = await PlaywrightAssert.ThrowsAsync<PlaywrightException>(() => page.Video.SaveAsAsync(""));
Assert.AreEqual("Page did not produce any video frames.", exception.Message);
await page.Video.DeleteAsync();
}
}
21 changes: 18 additions & 3 deletions src/Playwright/Core/Video.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,21 @@ internal class Video : IVideo
public Video(Page page, Connection connection)
{
_isRemote = connection.IsRemote;
page.Close += (_, _) => _artifactTcs.TrySetCanceled();
page.Crash += (_, _) => _artifactTcs.TrySetCanceled();
page.Close += (_, _) => _artifactTcs.TrySetResult(null);
page.Crash += (_, _) => _artifactTcs.TrySetResult(null);
if (page.IsClosed)
{
_artifactTcs.TrySetResult(null);
}
}

public async Task DeleteAsync()
{
var artifact = await _artifactTcs.Task.ConfigureAwait(false);
await artifact.DeleteAsync().ConfigureAwait(false);
if (artifact != null)
{
await artifact.DeleteAsync().ConfigureAwait(false);
}
}

public async Task<string> PathAsync()
Expand All @@ -52,12 +59,20 @@ public async Task<string> PathAsync()
throw new PlaywrightException("Path is not available when connecting remotely. Use SaveAsAsync() to save a local copy.");
}
var artifact = await _artifactTcs.Task.ConfigureAwait(false);
if (artifact == null)
{
throw new PlaywrightException("Page did not produce any video frames.");
}
return artifact.AbsolutePath;
}

public async Task SaveAsAsync(string path)
{
var artifact = await _artifactTcs.Task.ConfigureAwait(false);
if (artifact == null)
{
throw new PlaywrightException("Page did not produce any video frames.");
}
await artifact.SaveAsAsync(path).ConfigureAwait(false);
}

Expand Down

0 comments on commit 243b87f

Please sign in to comment.