Skip to content

fix(video): return pathlib.Path from Video.path() - #3197

Merged
Yury Semikhatsky (yury-s) merged 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/video-path-returns-path
Sep 23, 2026
Merged

Yury Semikhatsky (yury-s) merged 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/video-path-returns-path

Conversation

@MohammedAlkindi

@MohammedAlkindi Mohammed Alkindi (MohammedAlkindi) commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Video.path() is annotated -> pathlib.Path and the generated API documents Returns: pathlib.Path, but it returns Artifact.absolute_path, which is the raw initializer["absolutePath"] string.

The wrap was dropped in 0c70108 ("roll to ... aka video", Apr 2021) when Video moved onto the Artifact abstraction. Before that commit it did pathlib.Path(os.path.join(...)). The sibling Download.path() still wraps correctly via Artifact.path_after_finished(). Video.path() is the only consumer of absolute_path.

mypy does not catch it: ChannelOwner.__init__ takes a bare initializer: Dict, so absolute_path infers as Any and satisfies any declared return type. mypy playwright reports the same 2 pre-existing errors before and after.

Fail-before, real Chromium recording a real video:

assert isinstance('C:\\...\\page@a808d21e...webm', Path)
E   assert False

After: tests/async/test_video.py 5/5 and tests/sync/test_video.py 6/6 pass. Added the isinstance assertion to both so the contract is guarded; every existing assertion wrapped in str(...), which is why this stayed invisible.

pre-commit run --files on the three files: 15 hooks, 0 errors.

Video.path() is annotated -> pathlib.Path and the generated API documents it as such, but it returned Artifact.absolute_path, which is the raw initializer string. The wrap was dropped in 0c70108 when Video moved onto the Artifact abstraction; the sibling Download.path() still wraps via Artifact.path_after_finished().
@yury-s
Yury Semikhatsky (yury-s) merged commit ece7531 into microsoft:main Sep 23, 2026
35 checks passed
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

Successfully merging this pull request may close these issues.

2 participants