Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added base test #254

Merged

Conversation

alexandra-protyanova
Copy link
Contributor

@alexandra-protyanova alexandra-protyanova commented Jan 18, 2024

Screenshot 2024-01-22 at 15 28 24

Comment on lines +13 to +21
const baseCommand = 'node src/app.js';
let sourceContent = '';
const tempDir = path.join('tests', faker.word.noun());

const sourceFile = path.join(tempDir, faker.system.commonFileName('txt'));
const destinationFile = path.join(
tempDir,
faker.system.commonFileName('txt'),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason to have some constants inside the describe but the other's outside?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I based on the imports system which should be out of describe, but I read that it is commonly good practise to store requires out of describe

fs.rmdirSync(tempDir, { recursive: true });
});

test('should throw an error if only one argument is provided', async() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Students will start from success story
So it is more logical to test success flow first

test('should copy file to a new destination overwriting existing content', async() => {
const differentContent = faker.lorem.paragraph();

fs.writeFileSync(destinationFile, differentContent);
Copy link
Contributor

Choose a reason for hiding this comment

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

We use the same file all the time
What if prew tests block it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@alexandra-protyanova alexandra-protyanova merged commit b0b0065 into mate-academy:master Jan 23, 2024
1 check failed
@alexandra-protyanova alexandra-protyanova deleted the add-tests branch January 23, 2024 19:20
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