Skip to content

fix(tests): support Windows path separators in kernel pull tests#1095

Merged
stevemessick merged 2 commits into
Kaggle:mainfrom
sridipbasu:fix/windows-kernels-pull-tests
Jul 9, 2026
Merged

fix(tests): support Windows path separators in kernel pull tests#1095
stevemessick merged 2 commits into
Kaggle:mainfrom
sridipbasu:fix/windows-kernels-pull-tests

Conversation

@sridipbasu

Copy link
Copy Markdown
Contributor

Summary

This PR fixes platform-dependent assertions in the kernels_pull unit tests.

Problem

The tests in tests/unit/test_kernels_pull.py expected hardcoded POSIX-style paths (for example, /tmp/dummy/my-slug.py). On Windows, the production code constructs paths using os.path.join(), which produces Windows path separators. As a result, the tests failed even though the production code behaved correctly.

Solution

Updated the test assertions to construct the expected paths using os.path.join(), matching the behavior of the production code and making the tests platform-independent.

No production code was changed.

Testing

Verified on Windows by running:

python -m pytest tests/unit/test_kernels_pull.py -v

All four tests pass after the change.

Also ran:

python -m pytest tests/unit/ -v

All unit tests pass successfully.

@stevemessick

Copy link
Copy Markdown
Contributor

/gcbrun

Comment thread tests/unit/test_kernels_pull.py Outdated

# Verify file write
mock_open_file.assert_called_once_with("/tmp/dummy/my-slug.py", "w", encoding="utf-8")
expected_path = os.path.join("/tmp/dummy", "my-slug.py")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with #1091 (review), I find this confusing to read. If you're going to eliminate one slash then please remove them all. os.path.join("tmp", "dummy", "my-slug.py")

Also please note that we do not run tests on Windows during release testing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very Sorry about that. I ended up making the same mistake again. I've updated the path construction to use os.path.join(...) consistently throughout the tests and verified that everything still passes.

Thanks for pointing it out again, I’ll be more careful about keeping this consistent in future PRs.

@stevemessick

Copy link
Copy Markdown
Contributor

/gcbrun

@stevemessick
stevemessick merged commit cadeb92 into Kaggle:main Jul 9, 2026
12 checks passed
@stevemessick

Copy link
Copy Markdown
Contributor

Thanks!

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