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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bump to latest devtools #25

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

bump to latest devtools #25

wants to merge 7 commits into from

Conversation

paulirish
Copy link
Owner

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

@daniellara
Copy link

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

kimkha commented Feb 1, 2019

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

@springuper
Copy link

@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
Copy link

kimkha commented Dec 31, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants