Skip to content

[Java] Reformated Design Strategies to Use Codeblocks #2308

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

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

Conversation

shbenzer
Copy link
Contributor

@shbenzer shbenzer commented May 9, 2025

User description

Updated Design Strategies to use codeblock design for java examples and future

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

Enhancement, Documentation


Description

  • Refactored Java code examples in design strategies documentation to use code blocks

    • Replaced inline Java snippets with gh-codeblock references for maintainability
    • Ensured consistency across English, Japanese, Portuguese, and Chinese docs
  • Added comprehensive Java example file BestPractices.java for code block inclusion

    • Contains multiple Page Object and utility class examples for design strategies
  • Improved code snippet maintainability and translation readiness in documentation


Changes walkthrough 📝

Relevant files
Enhancement
BestPractices.java
Add comprehensive Java design strategies example file       

examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java

  • Added new Java file with multiple Page Object and utility class
    examples
  • Includes EditIssue, IssueList, EditIssueBetter, ProjectPage,
    SecuredPage, ActionBot, and test class
  • Designed for inclusion in documentation via code block references
  • +284/-0 
    Documentation
    design_strategies.en.md
    Refactor Java code examples to use code blocks in English doc

    website_and_docs/content/documentation/test_practices/design_strategies.en.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all relevant code tabs to use centralized code blocks
  • Improved maintainability and consistency of Java examples in
    documentation
  • Minor update to Python code block reference for completeness
  • +199/-319
    design_strategies.ja.md
    Refactor Java code examples to use code blocks in Japanese doc

    website_and_docs/content/documentation/test_practices/design_strategies.ja.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all Java code tabs for consistency and maintainability
  • Ensured translation structure remains intact
  • Minor update to Python code block reference for completeness
  • +199/-318
    design_strategies.pt-br.md
    Refactor Java code examples to use code blocks in Portuguese doc

    website_and_docs/content/documentation/test_practices/design_strategies.pt-br.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all Java code tabs for consistency and maintainability
  • Ensured translation structure remains intact
  • Minor update to Python code block reference for completeness
  • +199/-318
    design_strategies.zh-cn.md
    Refactor Java code examples to use code blocks in Chinese doc

    website_and_docs/content/documentation/test_practices/design_strategies.zh-cn.md

  • Replaced inline Java code snippets with gh-codeblock references to new
    Java file
  • Updated all Java code tabs for consistency and maintainability
  • Ensured translation structure remains intact
  • Minor update to Python code block reference for completeness
  • +199/-318

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @shbenzer shbenzer self-assigned this May 9, 2025
    Copy link

    netlify bot commented May 9, 2025

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 2a80d72

    Copy link
    Contributor

    qodo-merge-pro bot commented May 9, 2025

    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

    Method Parameter Error

    In the setSeleniumVersion method, line 48 uses logOutput variable instead of seleniumVersion parameter, which would cause incorrect data to be entered in the form.

    public void setSeleniumVersion(String seleniumVersion) {
      WebElement field = driver.findElement(By.id("issue_form_selenium-version"));
      clearAndType(field, seleniumVersion);
    }
    Incomplete Implementation

    The IssueList class extends LoadableComponent but doesn't implement the required load() and isLoaded() methods, which would cause runtime errors.

    public class IssueList extends LoadableComponent<IssueList> {
        private final WebDriver driver;
    
        public IssueList(WebDriver driver) {
            this.driver = driver;
        }
    
    }

    Copy link
    Contributor

    qodo-merge-pro bot commented May 9, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Implement required abstract methods
    Suggestion Impact:The suggestion was implemented by adding the required abstract methods load() and isLoaded() to the IssueList class. The class was moved to the end of the file and the methods were implemented, though with incorrect return types.

    code diff:

    +class IssueList extends LoadableComponent<IssueList> {
    +  private final WebDriver driver;
    +
    +  public IssueList(WebDriver driver) {
    +      this.driver = driver;
    +  }
    +
    +  @Override
    +  protected void load() {
    +    return true;
    +  }
    +
    +  @Override
    +  protected void isLoaded() throws Error {
    +    return true;
    +  }
    +
    +}

    The IssueList class extends LoadableComponent but doesn't implement the required
    abstract methods load() and isLoaded(). These methods are necessary for the
    LoadableComponent pattern to work correctly.

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java [77-84]

     public class IssueList extends LoadableComponent<IssueList> {
         private final WebDriver driver;
     
         public IssueList(WebDriver driver) {
             this.driver = driver;
         }
    -
    +    
    +    @Override
    +    protected void load() {
    +        // Implementation needed
    +    }
    +    
    +    @Override
    +    protected void isLoaded() throws Error {
    +        // Implementation needed
    +    }
     }

    [Suggestion processed]

    Suggestion importance[1-10]: 10

    __

    Why: The IssueList class extends LoadableComponent but doesn't implement the required abstract methods load() and isLoaded(), which would cause compilation errors. These methods are mandatory when extending the LoadableComponent class.

    High
    Fix generic type parameter
    Suggestion Impact:The commit changed the generic type parameter in EditIssueBetter class from EditIssue to EditIssueBetter, exactly as suggested

    code diff:

    -public class EditIssueBetter extends LoadableComponent<EditIssue> {
    +// class IssueList extends LoadableComponent<IssueList> {
    +//     private final WebDriver driver;
    +
    +//     public IssueList(WebDriver driver) {
    +//         this.driver = driver;
    +//     }
    +
    +// }
    +
    +class EditIssueBetter extends LoadableComponent<EditIssueBetter> {

    The generic type parameter in LoadableComponent is incorrect. It should be
    EditIssueBetter since this class is extending LoadableComponent for itself, not
    for the EditIssue class.

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java [86]

    -public class EditIssueBetter extends LoadableComponent<EditIssue> {
    +public class EditIssueBetter extends LoadableComponent<EditIssueBetter> {

    [Suggestion processed]

    Suggestion importance[1-10]: 9

    __

    Why: The generic type parameter is incorrect and would cause compilation issues. The class should extend LoadableComponent<EditIssueBetter> since it's implementing the LoadableComponent pattern for itself, not for the EditIssue class.

    High
    General
    Improve element existence check

    The code finds an element but doesn't use it, which is inefficient. Since you're
    only checking for existence, use findElements() and check the size instead of
    catching an exception, which is more performant.

    examples/java/src/test/java/dev/selenium/design_strategies/BestPractices.java [222-226]

    -try {
    -  WebElement div = driver.findElement(By.id("multilogin-dropdown"));
    -} catch (NoSuchElementException e) {
    +if (driver.findElements(By.id("multilogin-dropdown")).isEmpty()) {
       Assertions.fail("Cannot locate user name link");
     }
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: Using findElements() and checking if the list is empty is more efficient than catching a NoSuchElementException. This approach avoids the overhead of exception handling and follows best practices for element existence checks in Selenium.

    Medium
    • Update

    @shbenzer
    Copy link
    Contributor Author

    shbenzer commented May 9, 2025

    Better implementation of #2301

    @shbenzer
    Copy link
    Contributor Author

    Failures are unrelated to this pr

    @shbenzer shbenzer requested a review from harsha509 May 11, 2025 22:11
    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.

    1 participant