msramalho / SigTools

📆 Sigarra Tools | An extension that makes the information system of the University of Porto slightly better.
https://chrome.google.com/webstore/detail/sigarra-to-calendar/piefgbacnljenipiifjopkfifeljjkme
Apache License 2.0
37 stars 0 forks source link

Emulate `document` global object on unit tests #90

Closed fabiodrg closed 2 years ago

fabiodrg commented 3 years ago

Although most extractors use jQuery, there is code here and there that uses vanilla JS (my fault, I tend to prefer vanilla :stuck_out_tongue:), and therefore requires the standard Document interface for working with the DOM. At the moment, the Mocha unit tests setup the jQuery context for the HTML sample file under testing. I think it makes sense to set the global document variable as well, otherwise document is set for Mocha page: src/tests.html.

In principle, it shouldn't be that difficult. Just modify updatejQueryContext() on /src/test/setup.js, and when the jQuery AJAX request ends on .get(), set the global variable with an instance new DOMParser().parseFromString(html, 'text/html')) which creates the "normal" DOM object for the current sample page under testing.

fabiodrg commented 2 years ago

I was trying to solve this, but overriding global objects in the browser is not that easy after all, because window, document and others are write-protected. Besides, I could not find a way to mock those objects just for the purpose of running the unit tests and without affecting the report's DOM (where Mocha shows the results).

Most resources I find are Node.js specific, and from what I have seen it seems easier to mock in Node.js environment. Tthere are good modules that do the hard work and you get a full browser API in Node.js. And of course, it integrates easily with Mocka/Chai.

The question is if we really need to run the tests in a browser context or it's ok to do it in Node.js. I cant think of a limitation...

@msramalho any thoughts?

fabiodrg commented 2 years ago

So, while I was having my cup of coffee I had an idea, which is just a workaround. I suggested a transition to Node.js, so that the unit testing pipeline could be independent from a browser and it would be easier to mock protected global objects such document and window. Besides, ff anyone is into DevOps, it could use GitHub Actions more easily I believe Thus, I am convinced that would be a much better solution compared to my workaround.

But, that transition can take time because we need to make adaptions in all extractors to export functions/classes, setup a bundler so that the extension works when loaded in the browser, see if Firefox/Chrome complain with the packaging (we believe we had some issues with minification in the past), etc...

Thus, as a temporary workaround, I suggest the following:

I don't like to change the code base just for the sake of easing unit tests, but I think it is a simple workaround for the time being.