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

Skip test_get_container_error_timestamp on windows #3128

Merged

Conversation

eapolinario
Copy link
Collaborator

@eapolinario eapolinario commented Feb 12, 2025

Tracking issue

NA

Why are the changes needed?

The test test_get_container_error_timestamp has been failing on windows since its introduction. We don't get any signal from it, mainly because no one is really going to use pytorch on windows.

What changes were proposed in this pull request?

Skip a particular test on windows.

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Docs link

Summary by Bito

Added Windows-specific skip condition for test_get_container_error_timestamp test to address unreliable timestamp granularity issues. Implementation includes sys module import for platform detection and conditional test execution to handle persistent Windows test failures.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
@flyte-bot
Copy link
Contributor

flyte-bot commented Feb 12, 2025

Code Review Agent Run #cefaff

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: ee0a1f3..ee0a1f3
    • tests/flytekit/unit/bin/test_python_entrypoint.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

@flyte-bot
Copy link
Contributor

flyte-bot commented Feb 12, 2025

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Testing - Test Stability Improvement for Windows Platform

test_python_entrypoint.py - Added sys import and skip decorator for test_get_container_error_timestamp on Windows

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
@eapolinario eapolinario merged commit 0d0e01b into master Feb 12, 2025
108 of 110 checks passed
@flyte-bot
Copy link
Contributor

flyte-bot commented Feb 12, 2025

Code Review Agent Run #b5d975

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: ee0a1f3..eaecff4
    • tests/flytekit/unit/bin/test_python_entrypoint.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

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.

3 participants