search

paulirish / devtools-timeline-model

Unsupported
Other
173 stars 35 forks source link

bump to latest devtools #25

Open paulirish opened 7 years ago

paulirish commented 7 years ago

the bottom-up and topdown results aren't sorted correctly. aside from that, this is in good shape.

daniellara commented 6 years ago

Hi @paulirish I have passed several days trying to figure out how to fix the test in order to merge the PR because I need devtools-timeline-model updated to latest chrome version (or at least 67+).

Debugging devtools-timeline-model in master and latestbump branches I have reached these conclusions:

In this test:

it('metrics returned are expected', () => {
    assert.equal(model.timelineModel().tracks().filter(e => e.forMainFrame)[0].events.length, 7721);
    assert.equal(model.interactionModel().interactionRecords().length, 0);
    assert.equal(model.frameModel().frames().length, 16);
  });

You expected that the events of the main thread should be 7721 but it result in 7234. Debugging the test, I have seen that these events are conformed by the addition of Events, JSFrames and JSSamples. Between master and latetsbump there is a difference between JSFrames (1193 in master and 709 in latest, using the same report './test/assets/devtools-homepage-w-screenshots-trace.json').

So I think that the json cannot be the same between versions due to incompatibilities in chrome-devtools-frontend.

I also think that these test are testing that chrome-devtools-frontend is generating the model properly so in my opinion I suppose that should be this project instead devtools-timeline-model which has to test the model generation from file because devtools-timeline-model is simply using the API provided by chrom-devtools-frontend. So, ¿Is there any other approach to test this project like test that the API calls are returning values instead an exact value?

For example:

assert.ok(model.timelineModel().tracks().filter(e => e.forMainFrame)[0].events.length > 0);

Thanks in advance!

kimkha commented 5 years ago

This PR is awesome, and very worthy to merge. @paulirish, please merge it.

springuper commented 3 years ago

@paulirish when would you like to merge this PR? it would be very helpful for us so this great tool can support timeline file generated by latest Chrome.

kimkha commented 3 years ago

Looks like it's abandoned. Anyone interests to fork this repo and merge this PR?