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

All tests could also be running in the browser #532

Closed
edi9999 opened this issue Aug 9, 2018 · 8 comments · Fixed by #533
Closed

All tests could also be running in the browser #532

edi9999 opened this issue Aug 9, 2018 · 8 comments · Fixed by #533

Comments

@edi9999
Copy link
Contributor

edi9999 commented Aug 9, 2018

Some of the tests are not runned in browser environment :

For example : https://github.com/oliver-moran/jimp/blob/master/test/filetypes.test.js#L13

I think it would be possible to have a function that can read from the disk using one of :

fs.readFile(path)

fetch(path).then(function(response) {return response.text()}) with a static webserver running

What do you think of that @hipstersmoothie ?

I do what I describe in an other open-source library that I maintain : https://github.com/open-xml-templating/docxtemplater/blob/master/es6/tests/utils.js#L386 and it has been working quite well.

@edi9999
Copy link
Contributor Author

edi9999 commented Aug 9, 2018

Maybe instead of having a condition detecting if the build is in the browser we could even use a webpack-loader to load the pngs using require.

@hipstersmoothie
Copy link
Collaborator

hipstersmoothie commented Aug 9, 2018

I think we have something similar getJimpDir gets images and fonts served in the browser. The test you cite writes back to the filesystem, something I don't think we can do.

(accidently closed!)

@edi9999
Copy link
Contributor Author

edi9999 commented Aug 9, 2018

Indeed, the test that I pinpointed in this issue writes to disk, so this one is really not movable.

Some of the other tests could run in the browser too I think : https://github.com/oliver-moran/jimp/blob/master/test/async.test.js#L44

@hipstersmoothie
Copy link
Collaborator

Also I don't believe we should be doing any webpacking in jimp. Libraries like jimp should not be webpacked I beleive.

https://twitter.com/HipsterSmoothie/status/1023034485749637120

@hipstersmoothie
Copy link
Collaborator

I am ES6ing the tests right now. I'll unblock the getBuffer and getAync in the browser.

@hipstersmoothie
Copy link
Collaborator

async/await make the tests so much more readable

@edi9999
Copy link
Contributor Author

edi9999 commented Aug 9, 2018

Ok, good !

@hipstersmoothie hipstersmoothie mentioned this issue Aug 9, 2018
4 tasks
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 a pull request may close this issue.

2 participants