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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emulate document global object on unit tests #90

Closed
fabiodrg opened this issue Mar 13, 2021 · 2 comments
Closed

Emulate document global object on unit tests #90

fabiodrg opened this issue Mar 13, 2021 · 2 comments
Assignees
Labels
discuss good first issue Good for newcomers test Unit test

Comments

@fabiodrg
Copy link
Collaborator

Although most extractors use jQuery, there is code here and there that uses vanilla JS (my fault, I tend to prefer vanilla 馃槢), 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 fabiodrg added good first issue Good for newcomers test Unit test labels Mar 13, 2021
@fabiodrg fabiodrg self-assigned this Oct 30, 2021
@fabiodrg
Copy link
Collaborator Author

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
Copy link
Collaborator Author

fabiodrg commented Nov 1, 2021

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:

  • Add a getter document/window that by default simply returns returns the respective global object.
get document() {
        // 茅 lidar
        return document;
}
  • In the updatejQueryContext function, we override the getter so that it returns the newly created DOM from parsing the HTML file:
function updatejQueryContext(url) {
    return new Promise((resolve) => {
        $.get(url, function(html) {
            const dom = new DOMParser().parseFromString(html, 'text/html');
            // override here
            Object.defineProperty(Extractor.prototype, "document", {
                get: function () {
                    return dom;
                }
            });
            ...
        })
    });
}
  • For those who want to use vanilla APIs, e.g. document.querySelector, and want to create unit tests, just need to update their Extractor and replace document by this.document.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss good first issue Good for newcomers test Unit test
Projects
None yet
Development

No branches or pull requests

1 participant