Skip to content

Fixing ES issues and dependencies#6

Merged
tschoem merged 1 commit intomainfrom
eso-fix
Sep 3, 2025
Merged

Fixing ES issues and dependencies#6
tschoem merged 1 commit intomainfrom
eso-fix

Conversation

@tschoem
Copy link
Copy Markdown
Contributor

@tschoem tschoem commented Sep 1, 2025

Issues fixed for glob, rimraf
Adding support for node >=18.20
Picks up the sdk directly from local install rather than potential globally installed version

Issues fixed for glob, rimraf
Adding support for node >=18.20
Copy link
Copy Markdown

@abdul-qadir92 abdul-qadir92 left a comment

Choose a reason for hiding this comment

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

Review Summary

Approved - This PR successfully addresses the ES module compatibility issues and Node 18.20+ support goals.

What Works Well

  • ES Module Migration: Successfully updated to use named exports { rimrafSync } and { globSync } for compatibility with newer versions
  • Local SDK Resolution: Direct path to local .bin directory ensures the SDK is picked up from local install rather than global
  • Node 18.20+ Support: Added engines field and confirmed working with Node 18.20.0
  • Dependency Updates: Updated browserstack-node-sdk, selenium-webdriver, and other dependencies

Test Results

Tested on Node 18.20.0 with real .side file and browserstack.yml:

  • ✅ File processing and test generation works
  • ✅ BrowserStack integration successful (ran tests on 3 platforms)
  • ✅ Cleanup functionality working (removes _generated directory)
  • ✅ Error handling provides detailed failure reports

Non-blocking Recommendations

These are minor improvements that could be addressed in a follow-up PR:

  1. Mocha Flag: Use --timeout (singular) instead of --timeouts for correctness
  2. UTF-8 Encoding: Add 'utf8' parameter to fs.readFileSync() calls for JSON parsing reliability on Node 18+
  3. Commander Modernization: Consider updating to { program } import for modern ESM API (optional)

Verdict

The core functionality is solid and meets all the PR goals. The runner works correctly with Node 18.20.0 and successfully processes Selenium IDE files. Ready to merge! ��

@tschoem tschoem merged commit b290ddc into main Sep 3, 2025
7 checks passed
@tschoem tschoem deleted the eso-fix branch September 3, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants