Unit tests for Freeze Dry Module

[reficul31]

Refs The tests are not final. A few inputs on the testing methods would be useful. Also jest-mock-fetch is a really good library, given the choice I would like to keep it as a dependency. Saying that I know it messes up the repository with one ugly file containing just a single line, but hey I am open to other approaches.

One of the tests is failing. Specifically the fix-links tests to check the base element was added to the head or not. Any input on why it is failing would be really helpful. Otherwise a general review of the code is needed badly.

[reficul31]

The changes asked by @Sanjo have been impelmented. Although I am not sure of mock returning a small image in the data uri as I am currently very hazy on the implementation. It would be really helpful if we could work on the failing test as currently I have no idea why its failing. If any other changes are required feel free to review.

[Treora]

I tried running the tests but I get another error than you or Travis get:

  ● Test suite failed to run

    SyntaxError: Unexpected token [

      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/ScriptTransformer.js:289:17)
      at Object.<anonymous> (setupJest.js:1:118)

Apparently it does not like >ES6 syntax in jest-fetch-mock, though it does not mind it in the tests themselves (which I suppose do get transpiled while dependencies do not). Any idea how this could go wrong for me (also after a fresh npm install) but not on e.g. Travis?

[Treora]

As for the error with adding the base tag, the travis log reports this error: TypeError: head.insertAdjacentElement is not a function

I guess this could be an underlying bug in the jsdom.. (update: I now see insertAdjacentElement is not very standardised yet (see MDN), so it would not be surprising if it is not implemented in jsdom at all)

[reficul31]

@Treora I looked into jsdom implementation of the insertAdjacentElement, it seems it is not yet implemented. I guess a modification to the test to by mocking the function call might solve the problem? Also, still looking into your other error, I can't seem to figure out why it is happening at your end, also it is kinda hard to replicate the error since I haven't encountered it myself. But I'll still keep looking into it.

[Treora]

I just filed an issue at jsdom. In the meantime, we could either skip the test or mock the call and check if it gets the expected arguments.

[Treora]

@reficul31: I also found my problem with syntax errors: I used node 5, which did not eat the es6/7(?) syntax used by jest-fetch-mock. I don't think requiring people to use node >=6 is a problem.

[reficul31]

@Treora Thanks for looking into the error, so do you think using jest-fetch-mock is worth the trouble or not? I'll modify the tests according to whether we are able to use fetch-mock or not.

[ghost]

I think requiring node.js >= 6 is not a problem. We should document that in the readme "Running tests" section (doesn't exist yet). Fixing that would also be fairly easy (just add babel transpilation to the package).

[ghost]

Regarding the Element.insertAdjacentElement implementation: Element.insertAdjacentHTML is already implemented in jsdom. The Element.insertAdjacentElement implementation is very similar. So we can implement it fairly easily. Side node: Jest currently uses jsdom 9.x.x. The newest release is 11.x.x which does only support node.js >= 6.

[reficul31]

@Sanjo I can't seem to make the test work. So, I took to mocking the function call. I'll keep working out a way to work on the test so that we can shift our focus from that test and work on other test's implementation.

[reficul31]

@Treora @Sanjo Any other changes required?

[Treora]

@reficul31: Sorry I have not gotten around to reviewing your tests; either these or #109. I am not experienced with writing tests either, so I would have to sit down, look at the code itself again, and think what would actually be relevant scenarios to cover. If any of the mentors have thought about this too, I would be glad to hear their conclusions.

[Treora]

I committed the changes we made in our call in the test-freezedry branch (commit 314f069). You could incorporate them in this PR, possibly still change things if you wish to, but if you think it's good then I'll merge.