[Jest] Migrate src/ui/control/attribution_control.test.ts

[astridx]

Special things here:

1. I need to make changes in one file in the folder test/util/index.js. As this file is used also with tape I did not want to change it. I decide to copy it. cp test/util/index.js src/util/test/util.ts.
2. I created the setup file setup.js instead of the stubloader. (https://jestjs.io/docs/configuration#setupfilesafterenv-array). Then I copied the needed stuff form stubloader,.
3. I changed one source file. https://stackoverflow.com/a/62281432/4260616
4. I didn't change much in the test file itself.

If saw this error

    ReferenceError: setImmediate is not defined

      61 |
      62 |     postMessage(data: any) {
    > 63 |         setImmediate(() => {
         |         ^
      64 |             try {
      65 |                 for (const listener of this.postListeners) {
      66 |                     listener({data, target: this.target});

      at MessageBus.postMessage (src/util/web_worker.ts:63:9)
      at Actor.send (src/util/actor.ts:75:21)
      at src/util/dispatcher.ts:47:19

[github-actions]

Bundle size report:

Size Change: 0 B
Total Size Before: 194 kB
Total Size After: 194 kB

Output file	Before	After	Change
maplibre-gl.js	185 kB	185 kB	0 B
maplibre-gl.css	9.49 kB	9.49 kB	0 B

ℹ️ View Details

No major changes

[HarelM]

Please add the removal of the "old" file to #458, also setTimeout should be with 0 parameter, shouldn't it?

[astridx]

Please add the removal of the "old" file to #458 ...

It is already in #458. It is Point 5.

[astridx]

..., also setTimeout should be with 0 parameter, shouldn't it?

The delay is an optional parameter and is set 0 by default (https://developer.mozilla.org/en-US/docs/Web/API/setTimeout). But you are right. I added the paramater, so it is more correct.

[HarelM]

jest_setup? mock_setup? test_setup? I think setup.is too general...

[HarelM]

Thinking about it more, why not wrap the functionality in this file and move it to util.ts file and simply call it in beforeEach in the relevant tests? wouldn't it be more clear when looking at the test what the test is doing? It will cause a bit more code duplication, but I think it's OK...

[astridx]

I have now removed setup.js and moved the implementation to util.js and beforeEach() as suggested.

I am just trying to understand this more clearly. I thought the line window.performance.mark = jest.fn(); is necessary because of sinon. But I still don't understand exactly why in this test?

[HarelM]

I don't think it is related to sinon, I think it is related to node and the fact that the code runs in jsdom.
I'm guessing that somewhere in the code that the tests in this file go through they call window.performance.mark, and so you have to mock it for the tests to not fail on this.
I'm see the same with fetch so I'm doing global.fetch = null to solve this, which is very similar...