SeleniumHQ / selenium-ide

Open Source record and playback test automation for the web.
https://selenium.dev/selenium-ide/
Apache License 2.0
2.8k stars 762 forks source link

some qol changes #1837

Closed toddtarsi closed 3 months ago

toddtarsi commented 3 months ago

User description

Adds more variables to commands


PR Type

Enhancement, Tests


Description


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

    codiumai-pr-agent-pro[bot] commented 3 months ago

    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
    codiumai-pr-agent-pro[bot] commented 3 months ago

    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]](https://github.com/SeleniumHQ/selenium-ide/pull/1837/files#diff-9e4aa485e596920bbae5fa60cfc15763794c4d5bfb1801ae935cf450fb4c61ffR1888-R1892) ```diff +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]](https://github.com/SeleniumHQ/selenium-ide/pull/1837/files#diff-9e4aa485e596920bbae5fa60cfc15763794c4d5bfb1801ae935cf450fb4c61ffR1919-R1923) ```diff 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]](https://github.com/SeleniumHQ/selenium-ide/pull/1837/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR277-R277) ```diff +'@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]](https://github.com/SeleniumHQ/selenium-ide/pull/1837/files#diff-1c4e85faca72f94f0c9994e3de6ff078053a0d5268b0cbcbb952589059714369R19-R34) ```diff { "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]](https://github.com/SeleniumHQ/selenium-ide/pull/1837/files#diff-e081d46cb4dad99f14e00be21fa4c3551eb01234a0fc43600989ffc14538606bR30-R30) ```diff +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]](https://github.com/SeleniumHQ/selenium-ide/pull/1837/files#diff-1c4e85faca72f94f0c9994e3de6ff078053a0d5268b0cbcbb952589059714369R5-R88) ```diff +{ + "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