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

docs: improve consistency and readability of the guide #2168

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

kathyrollo
Copy link
Contributor

@kathyrollo kathyrollo commented Feb 15, 2025

User description

Hi @diemol , this is a refinement of my previous contributions for Page Component Objects (#1387, #1394).

Description

  • Add emphasis on the relationship between Page Objects and Page Component Objects.
  • Add a descriptive message to the error thrown, which informs the tester of the faulty strategy (predicate) passed.

Motivation and Context

  • The changes highlight Page Objects and Page Component Objects are used together, not one over the other. While this was previously implied, it was not obviously explicit.
  • A specific error message clearly identifies the error with the test rather than actual programming error, compared to a generic Exception message thrown.
  • Overall consistency of guide is improved, so the idea remains cohesive in all sections of the article.

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

Documentation


Description

  • Improved consistency and terminology in the Page Object Model guide.

  • Added emphasis on the relationship between Page Objects and Page Component Objects.

  • Enhanced error messages for better clarity during test failures.

  • Refined examples and explanations for better readability and understanding.


Changes walkthrough 📝

Relevant files
Documentation
page_object_models.en.md
Refined Page Object Model guide for clarity and consistency

website_and_docs/content/documentation/test_practices/encouraged/page_object_models.en.md

  • Replaced "Page Object" with "Page Object Model" for consistency.
  • Added clarifications on Page Component Objects and their relationship
    with Page Objects.
  • Improved error message for better debugging during test failures.
  • Enhanced explanations and examples for better readability and
    understanding.
  • +18/-15 

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link

    netlify bot commented Feb 15, 2025

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit a89b5e0

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Error Message

    The new error message "Product not found" is generic and could be more descriptive. Consider including details about the failed condition/predicate to help with debugging.

    .orElseThrow(() -> new RuntimeException("Product not found")); // Error thrown during actual test run

    Copy link
    Contributor

    qodo-merge-pro bot commented Feb 15, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Improve error message with context

    The error message in the RuntimeException should include more context about the
    search condition to help with debugging. Include details about the filter
    condition that failed to match.

    website_and_docs/content/documentation/test_practices/encouraged/page_object_models.en.md [285]

    -.orElseThrow(() -> new RuntimeException("Product not found")); // Error thrown during actual test run
    +.orElseThrow(() -> new RuntimeException("Product not found. Filter condition: " + condition.toString())); // Error thrown during actual test run
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: Adding the filter condition to the error message would significantly improve debugging capabilities by providing more context about why the product search failed, making it easier to identify and fix issues during test failures.

    Medium

    Copy link
    Member

    @harsha509 harsha509 left a comment

    Choose a reason for hiding this comment

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

    HI @kathyrollo ,

    Thank you for the PR.

    Could you please add/update similar changes in other translated pages too( pt-br.md,
    zh-cn.md, ja.md), which are other than in en(english) ?

    Thanks,
    Sri

    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.

    2 participants