Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

CI: add mochitests using the debugger and located in non-debugger test suites#5197

Merged
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
juliandescottes:add-ci-mochitests
Jan 26, 2018
Merged

CI: add mochitests using the debugger and located in non-debugger test suites#5197
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
juliandescottes:add-ci-mochitests

Conversation

@juliandescottes
Copy link
Copy Markdown
Member

WIP.

Added all the tests I could find the devtools that rely on the new debugger. In theory they should all be tested as well. If it increases the testing time too much, we can look in more details on try to remove tests that seem redundant regarding their usage of the debugger.

Let's see what the CI says.

Copy link
Copy Markdown
Contributor

@jasonLaster jasonLaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Comment thread bin/ci/run-tests.js Outdated
"devtools/client/framework/test/browser_source_map-01.js",
"devtools/client/framework/test/browser_source_map-absolute.js",
"devtools/client/framework/test/browser_source_map-init.js",
"devtools/client/framework/test/browser_source_map-inline.js",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the source-map tests all using the debugger?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They need a toolbox and to do so they open it on the debugger. I assumed they needed to start the debugger for the tests to work. But starting the toolbox on any other tool seems to be sufficient to make the tests pass. I'll remove those.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not include them, because a smaller list is easier...

we can always re-consider if they fail in a release :P

Comment thread bin/ci/run-tests.js
"devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_eval_in_debugger_stackframe1.js",
"devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_eval_in_debugger_stackframe2.js",
"devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_location_debugger_link.js",
"devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_stacktrace_location_debugger_link.js",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any inspector tests?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes there seems to at least be one: devtools/client/inspector/markup/test/browser_markup_links_06.js not sure how I missed it my first pass. Thanks!

Comment thread bin/ci/run-tests.js

const otherTests = [
"browser_net_open_in_debugger.js"
"devtools/client/framework/test/browser_browser_toolbox_debugger.js",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes!!

@juliandescottes
Copy link
Copy Markdown
Member Author

Fixed a typo + applied comments

@juliandescottes
Copy link
Copy Markdown
Member Author

We should have at least one failure here, we will need a m-c changeset with my patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1432842 before browser_browser_toolbox_debugger.js can pass.

@jasonLaster jasonLaster merged commit 117288a into firefox-devtools:master Jan 26, 2018
@juliandescottes
Copy link
Copy Markdown
Member Author

I hadn't updated the PR by commenting out the failing test, so I don't know if the timeout is related to setup issues.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants