Introduce FitAddon tests

[leomoty]

Fixes #2167

I have a few questions:

• Can I rely on the default #terminal-container size?
• Is changing width good enough? should cover height as well?
• I noticed that if I try something stupid, like having a 1x1 pixel #terminal-container the fit will propose a negative cols. Is this expected behavior?

I want to remove the duplicated code to a helper function before merging.

[Tyriar]

I noticed that if I try something stupid, like having a 1x1 pixel #terminal-container the fit will propose a negative cols. Is this expected behavior?

I guess this is a bug, feel free to add a test for that and fix it (just prevent < 1x1 cell?)

[Tyriar]

This might be better as an absolute px value in case #terminal-container's container changes its size?

[leomoty]

Sounds good

[Tyriar]

Can you move all tests relating to just proposeDimensions into describe('proposeDimensions', () => {, and all fit ones into the same but 'fit'?

[leomoty]

Leave the no terminal test outside?

[Tyriar]

The no terminal test just uses proposeDimensions so it can live in there

[Tyriar]

Again it might be safer to use px

[leomoty]

How about this instead @Tyriar? it removes a bit of duplicated code, but we don't rely directly on the default width? I can revert otherwise.

I still need to add one test for the 1x1 case, will do after work.

Edit: 1008px is the equivalent of 100% in a 1024x768 screen (you have to account for margin, padding and so forth).

[Tyriar]

Looks great, thanks! Should be good to merge after the small width test.

[leomoty]

I don't understand why the MacOS integration tests failed, I kept the same timeout that exists elsewhere?

Also, regarding the FitAddon, it doesn't need to be smaller than 1x1 to break:

    it('small width', async function(): Promise<any> {
      await openTerminal();
      await loadFit(1);
      console.log(await page.evaluate(`window.fit.proposeDimensions()`));
    });

returns:

{ cols: -2, rows: 26 }

    it('small width', async function(): Promise<any> {
      await openTerminal();
      await loadFit(1);
      await page.evaluate(`window.fit.fit()`);
      console.log(await page.evaluate(`window.term.cols`));
      console.log(await page.evaluate(`window.term.rows`));
    });

returns:

2
26

What is the desired result?

[leomoty]

Checking the BufferService.ts, I would expect FitAddon to match MINIMUM_COLS and MINIMUM_ROWS defined there. Since that is what Terminal.resize() seems to do nonetheless.

[Tyriar]

@leomoty proposeDimensions should give back the same number as an actual resize would, you can just hardcode the constant into the fit addon as well, should be enough as changing the other constant will break the test.

[Tyriar]

I don't understand why the MacOS integration tests failed, I kept the same timeout that exists elsewhere?

Not sure, try narrow down which await is not returning? I'll requeue the build in case it's flakiness

[leomoty]

I have updated the loadFit to allow both width/height (defaults are based from demo's style.css).

Also updated FitAddon to respect MINIMUM_COLS and MINIMUM_ROWS.

Mac OS is still flaky, but since I only have linux to test, I am not sure why.

[Tyriar]

Looks good, I'll look into the mac test now

[Tyriar]

I couldn't repro the flakiness on my mac, I pushed a change to avoid loading the page every time though which makes the tests take about 25% of the time. Hopefully the flakiness is fixed magically 🤞

[Tyriar]

They passed 3 times so I guess the flakiness is gone 🤷‍♂

Thanks for this! 👍