fix: file not found issue fixed for build tools and newsroom video script #3422
Conversation
vishvamsinh28
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe changes in this pull request involve enhancements to the test setups for two test files: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3422 +/- ##
=======================================
Coverage 67.77% 67.77%
=======================================
Files 21 21
Lines 664 664
=======================================
Hits 450 450
Misses 214 214 ☔ View full report in Codecov by Sentry. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3422--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
tests/build-newsroom-videos.test.js (1)
16-16: Fix indentation to match project style.There's an indentation issue flagged by ESLint.
Apply this fix:
- outputFileSync(testFilePath, JSON.stringify({})); + outputFileSync(testFilePath, JSON.stringify({}));🧰 Tools
🪛 eslint (1.23.1)
[error] 16-16: Delete
····(prettier/prettier)
tests/build-tools.test.js (2)
40-42: Fix indentation for consistency.The implementation effectively solves the file-not-found issue. However, there are some formatting inconsistencies.
Apply this diff to fix the indentation:
- fs.outputFileSync(automatedToolsPath, JSON.stringify({})); -fs.outputFileSync(toolsPath, JSON.stringify({})); - fs.outputFileSync(tagsPath, JSON.stringify({})); + fs.outputFileSync(automatedToolsPath, JSON.stringify({})); + fs.outputFileSync(toolsPath, JSON.stringify({})); + fs.outputFileSync(tagsPath, JSON.stringify({}));🧰 Tools
🪛 eslint (1.23.1)
[error] 40-40: Replace
········with····(prettier/prettier)
[error] 41-41: Delete
····(prettier/prettier)
[error] 42-42: Replace
········with····(prettier/prettier)
40-42: Consider enhancing the file initialization robustness.While the current implementation works, we could make it more robust and maintainable.
Consider these improvements:
+ const emptyJson = JSON.stringify({}); + try { + await Promise.all([ + fs.outputFileSync(automatedToolsPath, emptyJson), + fs.outputFileSync(toolsPath, emptyJson), + fs.outputFileSync(tagsPath, emptyJson) + ]); + } catch (error) { + console.error('Failed to initialize test files:', error); + throw error; + } - fs.outputFileSync(automatedToolsPath, JSON.stringify({})); - fs.outputFileSync(toolsPath, JSON.stringify({})); - fs.outputFileSync(tagsPath, JSON.stringify({}));Benefits:
- Reuses the empty JSON string
- Adds error handling for initialization failures
- Parallel file writing for better performance
🧰 Tools
🪛 eslint (1.23.1)
[error] 40-40: Replace
········with····(prettier/prettier)
[error] 41-41: Delete
····(prettier/prettier)
[error] 42-42: Replace
········with····(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
tests/build-newsroom-videos.test.js(2 hunks)tests/build-tools.test.js(1 hunks)
🧰 Additional context used
🪛 eslint (1.23.1)
tests/build-newsroom-videos.test.js
[error] 16-16: Delete ····
(prettier/prettier)
tests/build-tools.test.js
[error] 40-40: Replace ········ with ····
(prettier/prettier)
[error] 41-41: Delete ····
(prettier/prettier)
[error] 42-42: Replace ········ with ····
(prettier/prettier)
🔇 Additional comments (2)
tests/build-newsroom-videos.test.js (2)
1-1: LGTM! The file initialization fix looks good.
The addition of outputFileSync and its usage to create an empty JSON file before tests is a good solution to prevent the "file not found" issue. This ensures that the base file exists while still maintaining all error test cases.
Also applies to: 16-16
Line range hint 28-77: Verify test coverage remains comprehensive.
The test suite maintains good coverage with tests for:
- Successful video data fetching and file writing
- Fetch errors
- Invalid API responses
- HTTP error status codes
- File write errors
The addition of file initialization doesn't affect these test cases as they continue to test their respective scenarios effectively.
Let's verify the test coverage:
✅ Verification successful
Test coverage is comprehensive and consistent with codebase patterns
The test suite for buildNewsroomVideos follows the established error handling patterns seen across the codebase:
- All file operation errors (ENOENT/EACCES) are consistently handled
- The function is only used within its dedicated test file
- Error handling patterns match other similar build scripts (build-tools, build-meetings, build-rss)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any other test files that might need similar file initialization
# Look for similar test patterns that might need the same fix
rg -l "ENOENT|file not found" "tests/"
# Check for other test files using the same buildNewsroomVideos function
rg -l "buildNewsroomVideos" "tests/"
Length of output: 284
Script:
#!/bin/bash
# Let's check the error handling patterns in these test files to ensure consistency
rg -A 5 "ENOENT|file not found" "tests/"
# Also check if buildNewsroomVideos is imported or referenced in other files
rg "buildNewsroomVideos" "tests/"
Length of output: 2861
|
/rtm |
anshgoyalevil
added
the
gsoc
This PR fixes the file not found issue for tests of build-tools and newsroom video script
Summary by CodeRabbit