Closed Bug 654023 Opened 11 years ago Closed 9 years ago

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

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: erikvvold, Assigned: evold)

References

Details

(Whiteboard: [scratchpad])

Attachments

(2 files, 2 obsolete files)

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.
Assignee: nobody → erikvvold
Whiteboard: [workspaces]
Duplicate of this bug: 654022
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?
Attachment #529473 - Flags: review?(rcampbell)
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?
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?
(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.
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.
Sounds good to me
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.
Attachment #529473 - Flags: review?(rcampbell)
Attachment #529473 - Flags: review?(dtownsend)
Attachment #529473 - Flags: review+
Comment on attachment 529473 [details] [diff] [review]
revealing the workspace menu item

Test?
Attachment #529473 - Flags: review?(dtownsend) → review-
(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.
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) ;)
Status: NEW → ASSIGNED
(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?
(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.
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?
(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
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.
(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!
I'm just going to wait until bug 593540 lands then.
Depends on: 593540
Attached patch updated patch, w/ WIP test files (obsolete) — Splinter Review
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.
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).
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_".
Attached patch updated patch, w/ WIP test files (obsolete) — Splinter Review
Attachment #537953 - Attachment is obsolete: true
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?
can someone please take a look at my test file? I need some help.
Hey Erik,

looks like this'll need review from Dave. Sorry for the delay!
Attachment #538163 - Flags: review?(dtownsend)
(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 on attachment 538163 [details] [diff] [review]
updated patch, w/ WIP test files

Don't think this is ready for review yet
Attachment #538163 - Flags: review?(dtownsend)
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)
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
Attachment #538163 - Attachment is obsolete: true
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.
Summary: Adding the Workspace 'Tools' menu item to the Error Console → Adding the Scratchpad 'Tools' menu item to the Error Console
Whiteboard: [workspaces] → [scratchpad]
(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
(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?
filter on PEGASUS
Priority: -- → P3
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?
Component: Developer Tools → Developer Tools: Console
QA Contact: developer.tools → developer.tools.console
(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.
> I'm about ready to WONTFIX this as the Error Console is pretty deprecated UI.
Awwww.
Assignee: erikvvold → evold
Depends on: 859647
The Error Console has completely changed, it no longer has any menu, so it seems pointless to pursue this now..
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.