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

Get the url from the result array in GeckoProfiler.stop #1999

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

canova
Copy link
Contributor

@canova canova commented Oct 12, 2023

Previously for gecko profiler commands we were getting the test url by running a script that returns document.documentURI. This was working fine for most cases. But when one of our tests actually navigates to a different page from the starting page, it gets the destination url instead of the initial url. Since we need the initial url, it's better not to get the url like this, and get it directly from the result array.

Note that chrome trace command does something similar here.

cc @soulgalore and @92kns
This fixes Bug 1858709

Previously for gecko profiler commands we were getting the test url by
running a script that returns `document.documentURI`. This was working
fine for most cases. But when one of our tests actually navigates to a
different page from the starting page, it gets the destination url
instead of the initial url. Since we need the initial url, it's better
not to get the url like this, and get it directly from the `result`
array.

Note that chrome trace command does something similar [here](https://github.com/sitespeedio/browsertime/blob/e3e5a46f6c9070c0ab66e543b85e60d021edc848/lib/core/engine/command/chromeTrace.js#L42).
@soulgalore
Copy link
Member

Cool, let me rerun the tests to see the unit tests come through. There are two other actions that will fail at the moment: Chromedriver/Chrome has some breaking changes in the upcoming 119 release (the current driver doesn't work with upcoming releases) so lets merge with those failing.

@canova
Copy link
Contributor Author

canova commented Oct 13, 2023

@soulgalore thanks! It looks like the unit tests are failing again but they are passing locally.

@soulgalore
Copy link
Member

Yeah, the tests always work on my machine too ;)

Something is flakey in that action I will have a look at it later on. Thanks for the PR @canova

@soulgalore soulgalore merged commit 1b6d75a into sitespeedio:main Oct 13, 2023
12 of 14 checks passed
@canova canova deleted the profiler-command-url branch October 13, 2023 14:10
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.

None yet

2 participants