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

Add test for file not found #9

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cristianvasquez
Copy link

I thought this component was not handling that part but it appears to work. :)
#8

@bergos
Copy link
Member

bergos commented Apr 26, 2022

@cristianvasquez can you replace the asyncThrows with assert.rejects? I guess getStream could be replaced with stream.resume(). Then there would be no need for new dependencies.

@cristianvasquez
Copy link
Author

const stream = fromFile('not_found.ttl')
stream.resume()

However, that will not throw exceptions. Is that expected?

@cristianvasquez
Copy link
Author

cristianvasquez commented Apr 26, 2022

I've added the (failing) test in to branch

@bergos
Copy link
Member

bergos commented May 7, 2022

.resume() works, but the error is thrown after the async function is finished. You can add this line to wait for the error:

const { promisify } = require('util')
const { finished } = require('readable-stream')

// line after stream.resume()
await promisify(finished)(stream)

it('should throw an error if the file extension is unknown', async () => {
await assert.rejects(
async () => {
fromFile('test.jpg')
Copy link
Member

Choose a reason for hiding this comment

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

A sync error is thrown and throws() should be used. An await would be required if an async error is thrown.

it('should throw an error if the media type is unknown', async () => {
await assert.rejects(
async () => {
fromFile('test.jpg', {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, a sync error is thrown.

@cristianvasquez
Copy link
Author

I did the changes as requested

thanks for the review, I learned some things.

@bergos bergos self-requested a review December 9, 2024 22:07
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