Add some tests for JS frame page fields in tooltips

[canova]

Follow-up of #2300. Added some tests for the JS frame tooltips to make sure that we have Page URL and iframe URL fields in place with the correct data.

[codecov]

Codecov Report

Merging #2311 into master will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2311      +/-   ##
==========================================
+ Coverage   86.22%   86.26%   +0.04%     
==========================================
  Files         204      204              
  Lines       15257    15257              
  Branches     3830     3830              
==========================================
+ Hits        13155    13162       +7     
+ Misses       1925     1920       -5     
+ Partials      177      175       -2

Impacted Files	Coverage Δ
src/components/tooltip/CallNode.js	80.24% <0%> (+8.64%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 347033d...e391ec0. Read the comment docs.

[canova]

I checked with both snapshot and getByText function in those tests. Probably one of them is enough but wanted to keep both of them. My reasoning was, getByText makes sure we have that URL in place(in case we miss to check the snapshot changes in the future) and snapshot makes sure that the URLs have the correct titles(Page URL-iframe URL).

[gregtatum]

Having targeted tests plus snapshots is great!

[gregtatum]

Thanks for following up with the test!

I'm marking as "requesting changes" , as I have a suggestion that I think will make the code more readable, and reproducible.

I would suggest adding a setup function, and breaking out things into a separated it() blocks. Specifically an it block for the existence check, and a separate for the snapshot. I was thinking it could look like this:

function setup (pageUrl: string, iframeUrl: string?) {
  const {
    profile,
    funcNamesDictPerThread: [funcNamesDict],
  } = getProfileFromTextSamples(`
    A
    Bjs
    Cjs
  `);
  const threadIndex = 0;
  // Add a parent and an iframe page to Pages array.
  profile.pages = [
    {
      browsingContextID: 1,
      innerWindowID: 111111,
      url: pageUrl,
      embedderInnerWindowID: 0,
    },
  ];

  if (iframeUrl) {
    profile.pages.push({
      browsingContextID: 1,
      innerWindowID: 123123,
      url: iframeUrl,
      embedderInnerWindowID: 111111,
    });
  }

  const { frameTable } = profile.threads[threadIndex];

  for (let i = 1; i < frameTable.length; i++) {
    frameTable.innerWindowID[i] = profile.pages[1].innerWindowID;
  }

  const callNodePath = ['A', 'Bjs', 'Cjs'].map(name => funcNamesDict[name]);
  const { dispatch, renderTooltip } = setup(profile);
  dispatch(changeSelectedCallNode(threadIndex, callNodePath));
  const renderResults = renderTooltip();
  
  return { ...renderResults, pageUrl, iframeUrl }
}

it('displays Page URL for iframe pages', () => {
  const pageUrl = "http://..."
  const iframeUrl = "http://...";
  const { getByText } = setup(pageUrl, iframeUrl);
  expect(getByText(iframeUrl)).toBeTruthy();
  expect(getByText(pageUrl)).toBeTruthy();
});

That way the fixture logic would be separate from the assertions, and the error messages would be more granular.

[gregtatum]

Optional: A lot of the logic for generating the test fixture is shared, it might be worth collecting it in a setup function in order to make the actual test simpler to read. Then the expect(getByText(pageUrl)).toBeTruthy(); assertion and the snapshot assertion could be separated out into separate tests.

[gregtatum]

Having targeted tests plus snapshots is great!

[canova]

@gregtatum Thanks for the recommendation. Changed the code and added a setup function. But since there was a setup function in that file already, added another describe block and put that function in it. Can you take a look at it again?

[gregtatum]

Thanks for the changes, the tests are really easy to read now! And of course thanks for following up with the nice tests. 👍