feat: Add cross-platform sleep utility for e2e preparation

Allan2000-Git commented 2 weeks ago

User description

Description

Implement cross-platform sleep functionality for e2e tests

Fixes #506

Dependencies

Installed ts-node package to transpile typescript to javascript

Future Improvements

N/A

Mentions

@rajdip-b

Screenshots of relevant screens

N/A

Developer's checklist

[x] My PR follows the style guidelines of this project
[x] I have performed a self-check on my work

If changes are made in the code:

[x] I have followed the coding guidelines
[x] My changes in code generate no new warnings
[ ] My changes are breaking another fix/feature of the project
[ ] I have added test cases to show that my feature works
[ ] I have added relevant screenshots in my PR
[x] There are no UI/UX issues

Documentation Update

[ ] This PR requires an update to the documentation at docs.keyshade.xyz
[ ] I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

enhancement, dependencies

Description

Implemented a cross-platform sleep utility to replace the sleep command with OS-specific commands, enhancing compatibility across different operating systems.
Updated the e2e:prepare script in package.json to utilize the new cross-platform sleep utility.
Added ts-node and tsx as development dependencies to support TypeScript execution.
Set the package type to module to align with modern JavaScript standards.

Changes walkthrough 📝

Relevant files

Enhancement

cross-platform-sleep.ts

Implement cross-platform sleep utility for OS-specific commands

apps/api/src/common/cross-platform-sleep.ts

Implemented a cross-platform sleep utility using execSync.

Added OS-specific commands for Windows, macOS, and Linux.

Included error handling for unsupported operating systems.

+22/-0

Dependencies

package.json

Update package.json for cross-platform sleep and dependencies

apps/api/package.json

Added ts-node and tsx as development dependencies.

Modified e2e:prepare script to use the new cross-platform sleep
utility.

Set package type to module.

+6/-2

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

codiumai-pr-agent-free[bot] commented 2 weeks ago

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

**🎫 Ticket compliance analysis ✅** **[506](https://github.com/keyshade-xyz/keyshade/issues/506) - Fully compliant** Fully compliant requirements: * Implemented OS-specific sleep commands for Windows (powershell), Linux and macOS

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 Security concerns

Command Injection:
The `seconds` variable is used directly in command string construction. While currently hard-coded, if this becomes configurable in the future, it could be vulnerable to command injection if not properly sanitized.

⚡ Recommended focus areas for review

Hard-coded Value
The sleep duration is hard-coded to 3 seconds. Consider making this configurable through an environment variable or command line argument. Error Handling
The error message from execSync is logged but could contain sensitive information. Consider sanitizing or limiting the error output.

codiumai-pr-agent-free[bot] commented 2 weeks ago

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add validation to ensure sleep duration is positive to prevent runtime errors ___ Add input validation to ensure the sleep duration is a positive number to prevent potential issues with invalid sleep times. [apps/api/src/common/cross-platform-sleep.ts [4-5]](https://github.com/keyshade-xyz/keyshade/pull/523/files#diff-3b6d516a655bfec660c526ae34e357a9dcc10d3f9438dbad23024dbdbbde93caR4-R5) ```diff const seconds = 3; +if (seconds <= 0) { + console.error('Sleep duration must be a positive number'); + process.exit(1); +} const os = platform(); ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Input validation is crucial for preventing runtime errors and ensuring the sleep utility functions correctly. This is especially important as the value could be provided via command line arguments.	8
Enhancement	Make sleep duration configurable via command line arguments instead of hardcoding the value ___ Make the sleep duration configurable by accepting it as a command line argument instead of hardcoding it to 3 seconds. This allows more flexibility when using the utility. [apps/api/src/common/cross-platform-sleep.ts [4-5]](https://github.com/keyshade-xyz/keyshade/pull/523/files#diff-3b6d516a655bfec660c526ae34e357a9dcc10d3f9438dbad23024dbdbbde93caR4-R5) ```diff -const seconds = 3; +const seconds = parseInt(process.argv[2]) \|\| 3; const os = platform(); ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: Making the sleep duration configurable improves the utility's flexibility and reusability, especially since it's being used in build scripts where different wait times might be needed.	7
Best practice	Maintain consistent string formatting style across error messages ___ Use template literals consistently for error messages to maintain code style uniformity. [apps/api/src/common/cross-platform-sleep.ts [13-14]](https://github.com/keyshade-xyz/keyshade/pull/523/files#diff-3b6d516a655bfec660c526ae34e357a9dcc10d3f9438dbad23024dbdbbde93caR13-R14) ```diff -console.error('Unsupported operating system'); +console.error(`Unsupported operating system: ${os}`); process.exit(1); ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 3 Why: While consistent string formatting improves code style uniformity, this suggestion has minimal impact on functionality. Adding the OS to the error message provides slightly better debugging information.	3

💡 Need additional feedback ? start a PR chat

Allan2000-Git commented 2 weeks ago

@rajdip-b It's working but giving below error. It should not actually because other files are also using import

import { execSync } from 'child_process' ^^^^^^ SyntaxError: Cannot use import statement outside a module

Possible solution: "module": "ESNext", "moduleResolution": "Node"

rajdip-b commented 2 weeks ago

Possible solution: "module": "ESNext", "moduleResolution": "Node"

may be you can try doing this

Allan2000-Git commented 2 weeks ago

This is causing a lot of troubles

It doesn't detect type as module
Even if it detects, lint file gives error saying that type commonjs is required
If i change to commonjs, other files gets affected

I do not have a solution.

rajdip-b commented 2 weeks ago

Ah man this is bad.

keyshade-xyz / keyshade