Skip to content

Conversation

@Delta456
Copy link
Member

@Delta456 Delta456 commented Nov 24, 2024

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Removes @pytest.mark.xfail_safari and @pytest.mark.xfail_firefox attribute for python cookie tests

Motivation and Context

As per the docs and tested locally the attribute is no longer needed and can be tested with Safari and FireFox

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix, Tests


Description

  • Removed @pytest.mark.xfail_firefox and @pytest.mark.xfail_safari decorators from cookie tests, allowing them to run on Firefox and Safari without expected failures.
  • This change reflects that the sameSite cookie attribute is now supported in these browsers, aligning with local testing results.

Changes walkthrough 📝

Relevant files
Tests
cookie_tests.py
Remove xfail markers for Firefox and Safari in cookie tests

py/test/selenium/webdriver/common/cookie_tests.py

  • Removed @pytest.mark.xfail_firefox decorator from tests.
  • Removed @pytest.mark.xfail_safari decorator from tests.
  • Tests can now run on Firefox and Safari without expected failures.
  • +0/-7     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Test Coverage
    Verify that sameSite cookie functionality works correctly in Firefox and Safari browsers before removing xfail markers

    @qodo-merge-pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @VietND96
    Copy link
    Member

    The common test has been removed from the test matrix for safari since it took 3 hours to complete.
    Do you have any idea to optimize it and add it back via

    - browser: safari
    ?

    @Delta456
    Copy link
    Member Author

    I will try to see what can be done.

    @Delta456
    Copy link
    Member Author

    Delta456 commented Dec 9, 2024

    I wasn't able to find a way to optimise this.

    A workaround for this can be simply skipping this in the CI like it's done for other tests I believe.

    Copy link
    Member

    @diemol diemol left a comment

    Choose a reason for hiding this comment

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

    Thank you, @Delta456!

    @diemol diemol merged commit d1f9874 into SeleniumHQ:trunk Dec 28, 2024
    1 check passed
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Mar 23, 2025
    …iumHQ#14794)
    
    [py] remove xfail attr for firefox and safari
    
    Co-authored-by: Diego Molina <[email protected]>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants