654023 - Adding the Scratchpad 'Tools' menu item to the Error Console

Status: RESOLVED WONTFIX

(DevTools :: Console, defect, P3)

Description

[Erik Vold]

The patch for this issue: https://github.com/robcee/workspace/pull/16 was undone in the patch for bug 650345 and needs to be re-implemented.

Comment 2

[Erik Vold]

Attachment: revealing the workspace menu item

Comment 3

[Erik Vold]

Comment on attachment 529473 [details] [diff] [review]
revealing the workspace menu item

Can you give this a review when you have a change please Rob?

Comment 4

[Erik Vold]

Note I just filed bug 654108 which would mean the patch here would need to be altered. Also it'd be nice to DRY this crud, but I'm not sure where I should put a new include file, any advice?

Comment 5

[Rob Campbell [:rc] (:robcee)]

I'll certainly give this a review when I get through my bugmail.

(In reply to comment #4)
> Note I just filed bug 654108 which would mean the patch here would need to be
> altered. Also it'd be nice to DRY this crud, but I'm not sure where I should
> put a new include file, any advice?

Not sure what you mean by DRY?

Comment 6

[Erik Vold]

(In reply to comment #5)
> (In reply to comment #4)
> > Note I just filed bug 654108 which would mean the patch here would need to be
> > altered. Also it'd be nice to DRY this crud, but I'm not sure where I should
> > put a new include file, any advice?
> 
> Not sure what you mean by DRY?

My patch adds code to console.js which just displays the workspace menu item, and this code is basically the same as code that was added to browser.js when Workspace was imported.

I'm thinking that a singleton should observe changes to the devtools.workspace.enabled pref and show/hide the Workspace menu items w/in browser windows, console windows, and w/e other windows that we may wish to display the Workspace menu item in.

This way we don't need to litter w/in the browser.js and console.js files and we will have a single observer.

Comment 7

[Rob Campbell [:rc] (:robcee)]

ok, that makes sense.

But! Before you head off on that path, I'd like to ask the toolkit module owner David Townsend what he thinks about adding a Workspace^WScratchpad reference to console.js.

Dave: in the Workspace extension, Erik added a Workspace menu item to the Error Console's Tools menu. The rationale is, if you see an error in the Error Console, you might want to do a little exploratory hacking in a Workspace (scratchpad) to find the problem.

Comment 8

[Dave Townsend [:mossop]]

Sounds good to me

Comment 9

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 529473 [details] [diff] [review]
revealing the workspace menu item

this works for me, but I'll request an additional review from David as I'm not a module peer.

We will want to change this to "Scratchpad" once the renaming patch lands from bug 653093.

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 529473 [details] [diff] [review]
revealing the workspace menu item

Test?

Comment 11

[Erik Vold]

(In reply to comment #10)
> Comment on attachment 529473 [details] [diff] [review] [review]
> revealing the workspace menu item
> 
> Test?

I'll try to figure out how to write one tonight.

Comment 12

[Rob Campbell [:rc] (:robcee)]

I have a patch in bug 593540 with some tests in it for the error console. You might want to take a look there.

(I kinda thought Mossop might ask for a test for this) ;)

Comment 13

[Erik Vold]

(In reply to comment #12)
> I have a patch in bug 593540 with some tests in it for the error console. You
> might want to take a look there.

Hmm that's difficult for me to understand, the unit tests in bug 636725 are making more sense to me, but I'm not sure how to run the tests in either bug..

I read https://developer.mozilla.org/en/Mozilla_automated_testing but I'm not sure which test framework is being used in the tests mentioned above. Can I run the tests above with one of these make cmds? or with some other cmd?

Comment 14

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #13)
> (In reply to comment #12)
> > I have a patch in bug 593540 with some tests in it for the error console. You
> > might want to take a look there.
> 
> Hmm that's difficult for me to understand, the unit tests in bug 636725 are
> making more sense to me, but I'm not sure how to run the tests in either bug..

yeah, the tests in bug 593540 are kind of tricky because of the extra files.

> I read https://developer.mozilla.org/en/Mozilla_automated_testing but I'm not
> sure which test framework is being used in the tests mentioned above. Can I run
> the tests above with one of these make cmds? or with some other cmd?

You should be able to run your tests with "make mochitest-browser-chrome". Part of the "magic" is setting up your test file with a browser_ prefix and creating an appropriate tests directory with Makefile.in like in that example I pointed you at.

See also, mozilla-central/browser/base/content/test for more examples.

Let me know if I can help.

Comment 15

[Erik Vold]

So the docs here https://developer.mozilla.org/en/Mozilla_automated_testing say that I should do a simple build with --enable-tests, so I tried $ make --enable-tests -f client.mk. That didn't work, but after reading https://developer.mozilla.org/En/Developer_Guide/Build_Instructions/Mac_OS_X_Prerequisites I wrote a .mozconfig with the following:

ac_add_options --enable-debug

was that what I had to do? because when I run 

$ make mochitest-browser-chrome
make: *** No rule to make target `mochitest-browser-chrome'.  Stop.

I get the above output only.

So what have I done wrong?

Comment 16

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to comment #15)
> So the docs here https://developer.mozilla.org/en/Mozilla_automated_testing
> say that I should do a simple build with --enable-tests, so I tried $ make
> --enable-tests -f client.mk. That didn't work, but after reading
> https://developer.mozilla.org/En/Developer_Guide/Build_Instructions/
> Mac_OS_X_Prerequisites I wrote a .mozconfig with the following:
> 
> ac_add_options --enable-debug
> 
> was that what I had to do? because when I run 
> 
> $ make mochitest-browser-chrome
> make: *** No rule to make target `mochitest-browser-chrome'.  Stop.
> 
> I get the above output only.
> 
> So what have I done wrong?

Try something like the following from the top-level directory in the repo, substituting the test and build output directories for your case:

TEST_PATH=toolkit/components/console/hudservice/tests/browser/ make -C obj-ff-dbg mochitest-browser-chrome

Comment 17

[Erik Vold]

Thanks Panos! that worked.

Now I'm wondering where abouts the console ui tests are located, everything in toolkit/components/console/hudservice/tests/browser/ seems to be for the web console.

Comment 18

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #17)
> Thanks Panos! that worked.
> 
> Now I'm wondering where abouts the console ui tests are located, everything
> in toolkit/components/console/hudservice/tests/browser/ seems to be for the
> web console.

Hey Eric,

That's why I pointed you at bug 593540. There aren't any Error Console tests in the tree right now. The test patches in that bug have the necessary makefiles and structure to add a chrome test for the Error Console. You should be able to incorporate it for this bug.

I wish it were easier!

Comment 19

[Erik Vold]

I'm just going to wait until bug 593540 lands then.

Comment 20

[Erik Vold]

Attachment: updated patch, w/ WIP test files

Comment 21

[Erik Vold]

when I run:

TEST_PATH=toolkit/components/console/tests/ make -C obj-ff-dbg mochitest-browser-chrome

with the patch in comment 20 I get:

mochitest-browser-chrome failed:
TEST-UNEXPECTED-FAIL | (browser-test.js) | No tests to run. Did you pass an invalid --test-path?
make: *** [mochitest-browser-chrome] Error 1

any idea why?

I figured the ok(browser, "we have ze browzer"); part would be a test that should be passing.

Comment 22

[:Gavin Sharp [email: [e-mail]]]

toolkit/components/console/tests/ doesn't exist. You probably want toolkit/components/console/hudservice/tests (or just toolkit/components/console/ since there are no other tests under that directory).

Comment 23

[:Gavin Sharp [email: [e-mail]]]

Oh, sorry, I didn't see the patch. scratchpad_ui.js needs to be renamed browser_scratchpad_ui.js, browser chrome test filenames must begin with "browser_".

Comment 24

[Erik Vold]

Attachment: updated patch, w/ WIP test files

Comment 25

[Erik Vold]

Thanks Gavin.

With the patch in comment 24 I'm getting the following error message:

$ TEST_PATH=toolkit/components/console/tests/ make -C obj-ff-dbg mochitest-browser-chrome

rm -f ./mochitest-browser-chrome.log && /usr/bin/python2.6 _tests/testing/mochitest/runtests.py --autorun --close-when-done --console-level=INFO --log-file=./mochitest-browser-chrome.log --file-level=INFO --symbols-path=./dist/crashreporter-symbols --test-path=toolkit/components/console/tests/  --browser-chrome
/usr/bin/python2.6: can't open file '_tests/testing/mochitest/runtests.py': [Errno 2] No such file or directory
make: *** [mochitest-browser-chrome] Error 2

Any idea what I've done wrong now?

Comment 26

[Erik Vold]

can someone please take a look at my test file? I need some help.

Comment 27

[Rob Campbell [:rc] (:robcee)]

Hey Erik,

looks like this'll need review from Dave. Sorry for the delay!

Comment 28

[Dave Townsend [:mossop]]

(In reply to Erik Vold from comment #25)
> Thanks Gavin.
> 
> With the patch in comment 24 I'm getting the following error message:
> 
> $ TEST_PATH=toolkit/components/console/tests/ make -C obj-ff-dbg
> mochitest-browser-chrome
> 
> rm -f ./mochitest-browser-chrome.log && /usr/bin/python2.6
> _tests/testing/mochitest/runtests.py --autorun --close-when-done
> --console-level=INFO --log-file=./mochitest-browser-chrome.log
> --file-level=INFO --symbols-path=./dist/crashreporter-symbols
> --test-path=toolkit/components/console/tests/  --browser-chrome
> /usr/bin/python2.6: can't open file '_tests/testing/mochitest/runtests.py':
> [Errno 2] No such file or directory
> make: *** [mochitest-browser-chrome] Error 2
> 
> Any idea what I've done wrong now?

Did you build with tests disabled perhaps? Can you run browser chrome tests without your patch applied? This looks more like an issue with your build environment than your patch.

Comment 29

[Dave Townsend [:mossop]]

Comment on attachment 538163 [details] [diff] [review]
updated patch, w/ WIP test files

Don't think this is ready for review yet

Comment 30

[Rob Campbell [:rc] (:robcee)]

whups, my bad.

trying to apply this patch, I got:

creating toolkit/components/console/tests/scratchpad/Makefile
Makefile:55: *** missing separator.  Stop.

during a build. Looks like that Makefile has something funky in it.

Converted tabs to spaces and it built fine.

(see attached)

Comment 31

[Rob Campbell [:rc] (:robcee)]

Attachment: scratchpad menu item

with that applied, and run, I get the following log:

*** Start BrowserChrome Test Results ***
TEST-INFO | checking window state
TEST-INFO | unknown test url | before wait for focus -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) about:blank desired window: ([object ChromeWindow]) chrome://browser/content/browser.xul child window: ([object Window]) about:blank
TEST-INFO | unknown test url | already focused
TEST-INFO | unknown test url | maybe run tests <load:true, focus:true> -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) about:blank desired window: ([object ChromeWindow]) chrome://browser/content/browser.xul child window: ([object Window]) about:blank
TEST-START | chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js | Exception thrown at chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js:28 - ReferenceError: TEST_URI is not defined
INFO TEST-END | chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js | finished in 13ms
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js | Found an unexpected tab at the end of test run: about:blank
TEST-INFO | checking window state
TEST-INFO | unknown test url | before wait for focus -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) about:blank desired window: ([object ChromeWindow]) chrome://browser/content/browser.xul child window: ([object Window]) about:blank
TEST-INFO | unknown test url | already focused
TEST-INFO | unknown test url | maybe run tests <load:true, focus:true> -- loaded: complete active window: ([object ChromeWindow]) chrome://browser/content/browser.xul focused window: ([object Window]) about:blank desired window: ([object ChromeWindow]) chrome://browser/content/browser.xul child window: ([object Window]) about:blank
TEST-INFO | chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js | Console message: [JavaScript Error: "redeclaration of var Cc" {file: "chrome://browser/content/content.js" line: 3}]

INFO TEST-START | Shutdown
Browser Chrome Test Summary
	Passed: 0
	Failed: 2
	Todo: 0

*** End BrowserChrome Test Results ***
INFO | automation.py | Application ran for: 0:00:10.986675
INFO | automation.py | Reading PID log: /var/folders/dh/gjdqc1d48xlddx008k8wv4xr0000gn/T/tmpouz7WJpidlog
WARNING | automationutils.processLeakLog() | refcount logging is off, so leaks can't be detected!

INFO | runtests.py | Running tests: end.
mochitest-browser-chrome failed:
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js | Exception thrown at chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js:28 - ReferenceError: TEST_URI is not defined
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/console/tests/scratchpad/browser_scratchpad_ui.js | Found an unexpected tab at the end of test run: about:blank
make: *** [mochitest-browser-chrome] Error 1

Comment 32

[Rob Campbell [:rc] (:robcee)]

It does sound like you might have something funky in your build config if you're not able to run tests. A couple of things to look for:

1. check for an ac_add_options --enable/disable-tests in your .mozconfig and remove it if present (tests are enabled by default on builds)
2. try running make mochitest-browser-chrome from inside your objdir.

e.g., cd obj-ff-dbg; TEST_PATH=toolkit/components/console/ make mochitest-browser-chrome

let us know if that gets you unstuck.

Comment 33

[Erik Vold]

(In reply to Rob Campbell [:rc] (robcee) from comment #32)
> 1. check for an ac_add_options --enable/disable-tests in your .mozconfig and
> remove it if present (tests are enabled by default on builds)

K, I did this, but still getting the error in comment 25

Comment 34

[Erik Vold]

(In reply to Dave Townsend (:Mossop) from comment #28)
> (In reply to Erik Vold from comment #25)
> > Thanks Gavin.
> > 
> > With the patch in comment 24 I'm getting the following error message:
> > 
> > $ TEST_PATH=toolkit/components/console/tests/ make -C obj-ff-dbg
> > mochitest-browser-chrome
> > 
> > rm -f ./mochitest-browser-chrome.log && /usr/bin/python2.6
> > _tests/testing/mochitest/runtests.py --autorun --close-when-done
> > --console-level=INFO --log-file=./mochitest-browser-chrome.log
> > --file-level=INFO --symbols-path=./dist/crashreporter-symbols
> > --test-path=toolkit/components/console/tests/  --browser-chrome
> > /usr/bin/python2.6: can't open file '_tests/testing/mochitest/runtests.py':
> > [Errno 2] No such file or directory
> > make: *** [mochitest-browser-chrome] Error 2
> > 
> > Any idea what I've done wrong now?
> 
> Did you build with tests disabled perhaps?

That could be, how can I check?


> Can you run browser chrome tests
> without your patch applied? This looks more like an issue with your build
> environment than your patch.

I did a hg revert --all and I get the same error as in comment 25

What steps should I be following?

Comment 35

[Rob Campbell [:rc] (:robcee)]

filter on PEGASUS

Comment 36

[Paul Rouget [:paul]]

How do you plan to add this button to the Console UI? In the input as an "expand" button? Or just as a new button in the toolbar?

Comment 37

[Rob Campbell [:rc] (:robcee)]

(In reply to Paul Rouget [:paul] from comment #36)
> How do you plan to add this button to the Console UI? In the input as an
> "expand" button? Or just as a new button in the toolbar?

note, that this bug refers to the Error Console and not the Web Console.

I'm about ready to WONTFIX this as the Error Console is pretty deprecated UI.

Comment 38

[Philip Chee]

> I'm about ready to WONTFIX this as the Error Console is pretty deprecated UI.
Awwww.

Comment 39

[Erik Vold]

The Error Console has completely changed, it no longer has any menu, so it seems pointless to pursue this now..