Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/test.yml-template
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Test

on:
pull_request:
branches: [ master ]

jobs:
build:

runs-on: ubuntu-latest

strategy:
matrix:
node-version: [20.x]

steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node-version }}
- run: npm install
- run: npm test
9 changes: 5 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"devDependencies": {
"@faker-js/faker": "^8.4.1",
"@mate-academy/eslint-config": "latest",
"@mate-academy/scripts": "^1.8.6",
"@mate-academy/scripts": "^2.1.3",
"eslint": "^8.57.0",
"eslint-plugin-jest": "^28.6.0",
"eslint-plugin-node": "^11.1.0",
Expand Down
69 changes: 68 additions & 1 deletion src/app.js
Original file line number Diff line number Diff line change
@@ -1 +1,68 @@
// write code here
/* eslint-disable no-console */
'use strict';

const fs = require('fs/promises');
const path = require('path');

async function moveFile() {
const args = process.argv.slice(2);

const originPath = args[0];
const pathToMove = args[1];

if (!originPath) {
console.error('Source file path was not provided');

return;
}
Comment on lines +13 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This error message is slightly misleading. At this point, the program is checking for the presence of a command-line argument, not for the existence of a file on the filesystem. A clearer message might be 'Source file path was not provided'.


if (!pathToMove) {
console.error('Destination path was not provided');

return;
}
Comment on lines +19 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar to the previous check, this error message could be more precise. It's checking for a missing command-line argument, not a path on the filesystem. Consider changing it to something like 'Destination path was not provided'.


if (originPath === pathToMove) {
return;
}

let sourceStat = null;

try {
sourceStat = await fs.stat(originPath);
} catch (e) {
console.error('File does not exist');

return;
}

if (sourceStat.isDirectory()) {
console.error('Error: Source is a directory');

return;
}

try {
let finalDest = pathToMove;

try {
const destStat = await fs.stat(pathToMove);

if (destStat.isDirectory()) {
finalDest = path.join(pathToMove, path.basename(originPath));
}
} catch (e) {
if (pathToMove.endsWith('/') || pathToMove.endsWith('\\')) {
console.error('Path does not exist');

return;
Comment on lines +56 to +58

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The requirement states that an error must be thrown if a destination directory does not exist. Logging an error and returning causes the program to exit with a success code (0), which incorrectly signals that the operation succeeded. You should throw new Error(...) here instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

How should I do this if Cypress tests don't support this option, and exiting via process.exit(1) doesn't work either?

Comment on lines +56 to +58

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the requirements, when a destination directory does not exist, an error must be thrown. Using console.error and returning from the function will cause the process to exit successfully (with code 0), which incorrectly indicates that the operation succeeded. Please throw new Error(...) here instead.

}
}

await fs.rename(originPath, finalDest);
} catch (error) {
console.error('Error:', error.message);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This catch block handles errors from fs.rename but suppresses them by only logging the message. Any error during the file move, such as a non-existent parent directory in the destination path, should cause the application to fail. An error should be thrown here to ensure the process exits with a non-zero status code, signaling a failure.

}
Comment on lines +63 to +65

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This catch block handles errors from both fs.access on the source path and fs.rename for the move operation. Both can throw an ENOENT error, but for different reasons (source not existing vs. destination directory not existing). By using one generic message 'File does not exist', you provide misleading information when the destination directory is missing. Consider separating the logic to handle these errors differently to provide more accurate feedback.

Comment on lines +63 to +65

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This catch block prevents the application from signaling a failure when an error occurs during the fs.rename operation (for example, if an intermediate directory in the destination path doesn't exist). By catching the error and only logging it, the script finishes with a success exit code. For a command-line tool, it's better to let the error propagate (by removing this try...catch block or re-throwing the error) so that the process exits with a non-zero status code, indicating failure.

}

moveFile();
Loading