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

some qol changes #1837

Merged
merged 1 commit into from
Jun 20, 2024
Merged

some qol changes #1837

merged 1 commit into from
Jun 20, 2024

Conversation

toddtarsi
Copy link
Contributor

@toddtarsi toddtarsi commented Jun 5, 2024

User description

Adds more variables to commands


PR Type

Enhancement, Tests


Description

  • Simplified slicing logic in composePreprocessors function.
  • Added multiple new command preprocessors for various assertions in webdriver.ts.
  • Updated versions and dependencies across multiple package.json files to 4.0.10.
  • Enhanced text comparison test with variables in text-comparison.side.
  • Updated pnpm-lock.yaml to reflect new package versions.

Changes walkthrough 📝

Relevant files
Enhancement
2 files
preprocessors.ts
Simplified slicing logic in `composePreprocessors` function.

packages/side-runtime/src/preprocessors.ts

  • Simplified slicing logic in composePreprocessors function.
+1/-1     
webdriver.ts
Added and updated command preprocessors for assertions.   

packages/side-runtime/src/webdriver.ts

  • Added multiple new command preprocessors for various assertions.
  • Updated existing preprocessors to include targetFallback.
  • +54/-2   
    Dependencies
    21 files
    package.json
    Bumped package version to 4.0.10.                                               

    packages/browser-info/package.json

    • Updated version to 4.0.10.
    +1/-1     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/code-export-csharp-commons/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/code-export-csharp-nunit/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/code-export-csharp-xunit/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +4/-4     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/code-export-java-junit/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/code-export-javascript-mocha/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/code-export-python-pytest/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/code-export-ruby-rspec/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package version to 4.0.10.                                               

    packages/get-driver/package.json

    • Updated version to 4.0.10.
    +1/-1     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/selenium-ide/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +13/-13 
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/side-api/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +5/-5     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/side-code-export/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package version to 4.0.10.                                               

    packages/side-commons/package.json

    • Updated version to 4.0.10.
    +1/-1     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/side-example-suite/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +4/-4     
    package.json
    Bumped package version to 4.0.10.                                               

    packages/side-migrate/package.json

    • Updated version to 4.0.10.
    +1/-1     
    package.json
    Bumped package version to 4.0.10.                                               

    packages/side-model/package.json

    • Updated version to 4.0.10.
    +1/-1     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/side-runner/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/side-runtime/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +6/-6     
    package.json
    Bumped package version to 4.0.10.                                               

    packages/side-testkit/package.json

    • Updated version to 4.0.10.
    +1/-1     
    package.json
    Bumped package and dependencies versions to 4.0.10.           

    packages/webdriver-testkit/package.json

    • Updated version to 4.0.10.
    • Updated dependencies to 4.0.10.
    +3/-3     
    pnpm-lock.yaml
    Updated lock file with new package versions.                         

    pnpm-lock.yaml

    • Updated all package versions and dependencies to 4.0.10.
    +46/-46 
    Tests
    1 files
    text-comparison.side
    Enhanced text comparison test with variables.                       

    tests/examples/text-comparison.side

  • Added new variables for ID and text values.
  • Updated commands to use variables.
  • +83/-50 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    added a bunch more command preprocessors (variable handling)
    made everything 4.0.10 so incrementing versions can just be dumb
    i mean gosh maintaining versions manually is so painful
    changed some side tests to use a bunch more variables
    Copy link

    qodo-merge-pro bot commented Jun 5, 2024

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    3, because the PR includes a variety of changes across multiple files, including logic adjustments in function calls, addition of new command preprocessors, and updates to package versions. The changes are not overly complex but are numerous and spread across different aspects of the project, requiring careful review to ensure compatibility and correctness.

    🧪 Relevant tests

    Yes

    ⚡ Possible issues

    Duplicate Function: The function WebDriverExecutor.prototype.doAssertNotChecked is defined twice in webdriver.ts. This could lead to unexpected behavior or errors during runtime.

    Version Consistency: The PR updates several package versions to 4.0.10. It's important to ensure that these updates do not introduce any compatibility issues with other parts of the system.

    🔒 Security concerns

    No

    Copy link

    qodo-merge-pro bot commented Jun 5, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Remove the duplicate definition of a function to avoid confusion and potential issues

    There is a duplicate definition of doAssertNotChecked in the new code. This could lead to
    unexpected behavior or confusion. Remove the duplicate definition to ensure clarity and
    correctness.

    packages/side-runtime/src/webdriver.ts [1888-1892]

    +WebDriverExecutor.prototype.doAssertNotChecked = composePreprocessors(
    +  interpolateString,
    +  { targetFallback: preprocessArray(interpolateString) },
    +  WebDriverExecutor.prototype.doAssertNotChecked
    +)
     
    -
    Suggestion importance[1-10]: 10

    Why: The suggestion correctly identifies a critical issue where a function is defined twice, which could lead to runtime errors or logical bugs. Removing the duplicate is crucial for the functionality of the code.

    10
    Possible bug
    Correct the function reference to ensure the correct functionality is applied

    The doAssertNotSelectedLabel function is incorrectly referencing doAssertSelectedLabel
    instead of doAssertNotSelectedLabel. This could lead to incorrect functionality.

    packages/side-runtime/src/webdriver.ts [1919-1923]

     WebDriverExecutor.prototype.doAssertNotSelectedLabel = composePreprocessors(
       interpolateString,
       interpolateString,
       { targetFallback: preprocessArray(interpolateString) },
    -  WebDriverExecutor.prototype.doAssertSelectedLabel
    +  WebDriverExecutor.prototype.doAssertNotSelectedLabel
     )
     
    Suggestion importance[1-10]: 10

    Why: This suggestion accurately points out a serious bug where the wrong function is referenced, potentially leading to incorrect behavior in the application. Correcting this ensures the function behaves as intended.

    10
    Best practice
    Ensure consistent dependency versions across all packages

    Ensure that all dependencies are updated consistently across all packages to avoid
    potential version conflicts and ensure compatibility.

    pnpm-lock.yaml [277]

    +'@seleniumhq/side-model':
    +  specifier: 4.0.10
    +  version: link:../side-model
     
    -
    Suggestion importance[1-10]: 8

    Why: This suggestion is important for maintaining consistency and compatibility across packages, which can prevent potential conflicts and ensure stable operation of the software.

    8
    Enhancement
    Add comments to store commands to provide context for stored values

    Consider adding a comment field for the store commands to provide context or explanation
    for storing the values ID, ACTUAL_TEXT, and NOT_ACTUAL_TEXT. This can help future
    maintainers understand the purpose of these commands.

    tests/examples/text-comparison.side [19-34]

     {
       "command": "store",
       "target": "b",
       "value": "ID",
    -  "id": "dadb224c-3564-43e5-9cfb-bb8be49a568e"
    +  "id": "dadb224c-3564-43e5-9cfb-bb8be49a568e",
    +  "comment": "Store the ID of the element"
     },
     {
       "command": "store",
       "target": "show alert",
       "value": "ACTUAL_TEXT",
    -  "id": "a0913ef7-8072-40e5-9b03-0e2adc98f5f9"
    +  "id": "a0913ef7-8072-40e5-9b03-0e2adc98f5f9",
    +  "comment": "Store the expected text to show alert"
     },
     {
       "command": "store",
       "target": "not show alert",
       "value": "NOT_ACTUAL_TEXT",
    -  "id": "eba5cd58-ef78-4524-a851-68e90db13157"
    +  "id": "eba5cd58-ef78-4524-a851-68e90db13157",
    +  "comment": "Store the expected text to not show alert"
     }
     
    Suggestion importance[1-10]: 7

    Why: Adding comments to the store commands would indeed improve the readability and maintainability of the code by providing context, which is beneficial for future maintainers.

    7
    Simplify the slice method usage for better readability

    The params variable can be simplified by using the slice method with a single argument,
    which is more concise and easier to read.

    packages/side-runtime/src/preprocessors.ts [30]

    +const params = args.slice(0, -1)
     
    -
    Suggestion importance[1-10]: 6

    Why: The suggestion improves code readability by simplifying the method call, although it's a minor enhancement and not critical to functionality.

    6
    Maintainability
    Format the JSON file with consistent indentation and spacing for better readability

    To improve readability, consider formatting the JSON file with consistent indentation and
    spacing. This will make it easier to read and maintain.

    tests/examples/text-comparison.side [5-88]

    +{
    +  "id": "1f270ca5-5a7c-4067-8f67-b4fc0dcea00b",
    +  "name": "text comparison",
    +  "commands": [
    +    {
    +      "id": "87af5840-26f1-4a35-8814-3d4e05c913a9",
    +      "comment": "",
    +      "command": "open",
    +      "target": "/popup/alert.html",
    +      "targets": [],
    +      "value": ""
    +    },
    +    ...
    +  ]
    +}
     
    -
    Suggestion importance[1-10]: 5

    Why: Consistent formatting improves readability, but the suggestion does not address a specific issue with the current formatting in the provided PR code diff.

    5

    @toddtarsi toddtarsi merged commit 6dbb969 into trunk Jun 20, 2024
    2 checks passed
    @toddtarsi toddtarsi deleted the 5-5-24-rc1 branch June 20, 2024 02:46
    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