bugGNU Octave - Bugs: bug #49119, libinterp/corefcn/debug.cc-tst...

 
 

bug #49119: libinterp/corefcn/debug.cc-tst unit tests cause several files to open in GUI editor

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Sat 17 Sep 2016 10:23:13 PM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Philip Nienhuis Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 20 Jul 2018 03:25:09 AM UTC, comment #27: 

I checked in a change (https://hg.savannah.gnu.org/hgweb/octave/rev/0214d9dabba2) which modifies the tests in debug.cc to use _octave_link_gui_preference_ to stop opening a file during the testing.  This was preferable to adding the code to test.m because it really, really slowed down the execution of BIST tests for which this was never an issue.

Marking bug as fixed and closing report.

Rik <rik5>
Group administrator
Thu 19 Jul 2018 07:44:30 PM UTC, comment #26: 

The function returns the value (as string) that was read before  the new value is set. This is for storing the previous value for restoring it afterwards. In addition, "value" is expected to be a string, regardless the type of the preference.

Torsten Lilge <ttl>
Group Member
Thu 19 Jul 2018 03:41:27 PM UTC, comment #25: 

The get functionality seems to work, but setting does not.


>> a = __octave_link_gui_preference__ ('editor/show_dbg_file')
a =
>> double (a)
ans =  1
>> a = __octave_link_gui_preference__ ('editor/show_dbg_file', false)
a =
>> double (a)
ans =  1
>> a = __octave_link_gui_preference__ ('editor/show_dbg_file', false)
a =
>> double (a)
ans =  1
>>



Rik <rik5>
Group administrator
Wed 18 Jul 2018 07:25:21 PM UTC, comment #24: 

Changeset http://hg.savannah.gnu.org/hgweb/octave/rev/7644e7f1721f provides the function


__octave_link_gui_preference__ (key, value)


which now can be used for setting the pref "editor/show_dbg_file" to "false" while storing the value returned by the function (the previous value) for restoring it later.

Torsten Lilge <ttl>
Group Member
Wed 18 Jul 2018 04:36:59 AM UTC, comment #23: 

I am currently working on an internal function


__octave_link_gui_preference__ (key, value)


which reads the current value for the preference "key" and sets this preference to "value" if "value" is not empty. The previous value for "key" is returned.

Torsten Lilge <ttl>
Group Member
Wed 18 Jul 2018 04:12:30 AM UTC, comment #22: 

I verified that with Torsten's most recent cset, and the GUI preference for opening an editor window when a breakpoint is set disabled, that no windows are created when runing 'test debug.cc'.

The remaining challenge is to code a way to toggle this preference from the Octave thread as suggested in comment #21.

Rik <rik5>
Group administrator
Sun 15 Jul 2018 05:07:31 PM UTC, comment #21: 

With cset http://hg.savannah.gnu.org/hgweb/octave/rev/a7ee69d23f32, I have added a gui preference for opening closed files if a breakpoint or a debug pointer is set. Regardless this preference, clearing a breakpoint never opens closed files. I hope this meets most of the expectations that were mentioned during the discussion on the behavior of the editor during debugging from the console window.

The next step will be a new internal function, such as, e.g., "__octave_link_gui_preferences__" for setting gui preferences from  within octave functions.

Torsten Lilge <ttl>
Group Member
Tue 03 Jul 2018 06:39:01 PM UTC, comment #20: 

I think this may reveal a fault in 'dbclear all'.  If the exact breakpoint is removed then I no longer see the behavior.


run-octave --gui -f
__octave_link_disable_breakpoints__
dbstop quantile>__quantile__
##dbclear all
dbstatus
dbclear quantile>__quantile__
__octave_link_enable_breakpoints__
clear -f




Rik <rik5>
Group administrator
Tue 03 Jul 2018 06:33:38 PM UTC, comment #19: 

Here is a minimal example which does not depend on test.


run-octave --gui -f
__octave_link_disable_breakpoints__
dbstop quantile>__quantile__
dbclear all
__octave_link_enable_breakpoints__
clear -f


It is when the function is cleared from memory that it appears in the editor.



Rik <rik5>
Group administrator
Tue 03 Jul 2018 06:18:32 PM UTC, comment #18: 

I can confirm the behavior of Mike's patch from comment #15.  'clear all'.  This can't be accidental.  The test code for debug.cc includes


%! dbstop @ftp/dir;
%! dbstop @audioplayer/set 70;
%! dbstop quantile>__quantile__;
%! dbstop ls;


What is particularly interesting is I re-ordered the calls to dbstop, and when I run 'clear all' I now get all 4 files.


%!test
%! dbclear all;   # Clear out breakpoints before test
%! dbstop ls;
%! dbstop @ftp/dir;
%! dbstop @audioplayer/set 70;
%! dbstop quantile>__quantile__;
%! s = dbstatus;
%!# keyboard;
%! dbclear all;
%! #assert (s(1).name, "@audioplayer/set>setproperty");
%! #assert (s(2).name, "@ftp/dir");
%! #assert (s(3).name, "ls");
%! #assert (s(4).name, "quantile>__quantile__");
%! #assert (s(2).file(end-10:end), [filesep "@ftp" filesep "dir.m"]);



Rik <rik5>
Group administrator
Tue 03 Jul 2018 05:57:56 PM UTC, comment #17: 

Emails crossed.  I see Torsten responded that it is also possible to do this on the GUI side.

Does anyone have a preference or justification for which side of the octave_link interface this should be implemented on?

Rik <rik5>
Group administrator
Tue 03 Jul 2018 05:54:53 PM UTC, comment #16: 

Correct.  There is no preference today, but I wanted to see if one could be added on the GUI thread side.

Otherwise, we can add a new preference variable on the interpreter side.  Should it be an internal setting (one with '__' prefix)?

For a template, see debug_on_error in error.cc


// TRUE means that Octave will try to enter the debugger when an error
// is encountered.  This will also inhibit printing of the normal
// traceback message (you will only see the top-level error message).
bool Vdebug_on_error = false;


and later


DEFUN (debug_on_error, args, nargout,
       doc: /* -*- texinfo -*-
@deftypefn  {} {@var{val} =} debug_on_error ()
@deftypefnx {} {@var{old_val} =} debug_on_error (@var{new_val})
@deftypefnx {} {} debug_on_error (@var{new_val}, "local")
Query or set the internal variable that controls whether Octave will try
to enter the debugger when an error is encountered.

This will also inhibit printing of the normal traceback message (you will
only see the top-level error message).

When called from inside a function with the @qcode{"local"} option, the
variable is changed locally for the function and any subroutines it calls.
The original variable value is restored when exiting the function.
@seealso{debug_on_warning, debug_on_interrupt}
@end deftypefn */)
{
  return SET_INTERNAL_VARIABLE (debug_on_error);
}


So we could have a Vdo_gui_breakpoints variable in octave-link.cc, move the breakpoint routines out of the .h file and in to the .cc file, and then condition calling through to octave_link based on this variable.

Rik <rik5>
Group administrator
Tue 03 Jul 2018 05:51:29 PM UTC, comment #15: 

I tested the attached change, which just adds an internal setting instead of a user-visible setting. But this can be extended to a user setting if someone likes.

However, I see a problem with this.

Running 'test libinterp/corefcn/debug.cc-tst' works, no files are opened in the editor. However, running 'clear all' after the test command returns results in 'quantile.m' and 'set.m' being opened in the editor, I'm not sure why that is.

(file #44500)

Mike Miller <mtmiller>
Group Member
Tue 03 Jul 2018 05:48:12 PM UTC, comment #14: 

Currently there is no such user preference, but looking at comment #7 and comment #8, I have the impression that there should be one. And yes I think it should be possible to change this from the interpreter thread (we can also open a file in the gui editor via "edit foo").

Torsten Lilge <ttl>
Group Member
Tue 03 Jul 2018 05:30:48 PM UTC, comment #13: 

I don't think there is a user preference for disabling this behavior. When 'dbstop' or 'dbclear' or other debug breakpoint commands are called at the command prompt, the corresponding file is automatically opened in the editor.

If someone thinks it would be useful to have a user setting, that can be done. But to solve this smaller problem it might be easier to add a setting to the octave_link class to selectively enable and disable the octave_link::update_breakpoint function.

As a test I commented out the octave_qt_link implementation of do_update_breakpoint and no files were opened in the editor.

Mike Miller <mtmiller>
Group Member
Tue 03 Jul 2018 04:14:52 PM UTC, comment #12: 

Is there a way to change a GUI preference (something stored under ~/.config/octave) from the interpreter thread?  I'm adding Torsten to the CC list since he will know.

If the answer is yes, then we could add a new GUI preference which controls whether setting a breakpoint in the Command Window automatically opens the file in the Editor window.  During the BIST test we could use an unwind_protect block to disable this setting and then restore it to whatever it was (most likely "on" since people seem to expect this).

Rik <rik5>
Group administrator
Wed 21 Sep 2016 06:41:51 PM UTC, comment #11: 

Yeah I suppose my thinking was too naive :-)  I overlooked that the tests are run by the interpreter, not by a binary fuction.

Well maybe we'll find a way someday.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 21 Sep 2016 03:24:13 PM UTC, comment #10: 

Is there a way to close an editor tab from the command line?  I thought it had to be done by using the mouse on the close icon, or a keyboard shortcut.

Rik <rik5>
Group administrator
Wed 21 Sep 2016 09:47:55 AM UTC, comment #9: 

Alternatively the files in the editor could be closed using the relevant function call after the tests in debug-cc.tst.
After all, that is what Octave does too upon exit when closing down the GUI.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 21 Sep 2016 12:33:57 AM UTC, comment #8: 

I think that is the desired behavior for users of the GUI, from what I've seen on bug tracker activity dealing with breakpoints and the GUI editor.

What's surprising is the test suite testing the debugging commands and having those files be opened and remain open in the editor after the test suite finishes. Maybe something could be added to temporarily disable this link during the running of this particular unit test. Not a very high priority, though, I agree.

Mike Miller <mtmiller>
Group Member
Wed 21 Sep 2016 12:24:40 AM UTC, comment #7: 

This was clearly an architectural decision, although I don't know if it was the right one.  Whenever a breakpoint is created from the command window, and the GUI is active, the corresponding file is loaded into the editor with the breakpoint set.

Simple test case:


./run-octave -f
dbstop ls


This behavior has been with us a while.  It goes back at least to version 3.8.2.

Is it worth changing?  Or is it kind of handy that the files you are debugging are automatically available in the GUI editor window?

Rik <rik5>
Group administrator
Tue 20 Sep 2016 04:01:08 PM UTC, comment #6: 

Opening a fresh Octave, with no files open in the editor, and running


test libinterp/corefcn/debug.cc-tst


shows 5 files left open in the editor.

Mike Miller <mtmiller>
Group Member
Tue 20 Sep 2016 03:58:41 PM UTC, comment #5: 

Thanks for pointing that out Dmitri. This is likely due to the unit tests in libinterp/corefcn/debug.cc. It tests setting and deleting debug breakpoints in help, ls, @audioplayer/set, @ftp/dir, and quantile.

Mike Miller <mtmiller>
Group Member
Tue 20 Sep 2016 03:32:26 PM UTC, comment #4: 

I see it on Fedora (see atached screenshot). Not only it opens
help.m, ls.m,dir.m,set.m, quantile.m it also sets some breakboints
(that is what red/orange dots are, aren't they?) in some of them.
Some breakponts get cleared off after the test run is finished.

Dmitri.
--



Dmitri A. Sergatskov <dasergatskov>
Tue 20 Sep 2016 02:11:35 PM UTC, comment #3: 

Rik, I didn't say that is mappers.cc that is at fault, rather that this "bug" happens around the time that the mappers-cc.tst tests run in __run_test_suite__ .
I am quite surprised that it happens during the tests of the binary (builtin) functions (at least, on Windows).

And funny that you can see it on Linux as well; I couldn't (Mageia-4 64bit)

I agree with lowering priority - unless this points to something that can bite later on. But that hasn't happened yet AFAICS.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 19 Sep 2016 11:02:53 PM UTC, comment #2: 

Also, I can reproduce this on Linux, so this isn't just an MS Windows problem.

Rik <rik5>
Group administrator
Mon 19 Sep 2016 10:54:41 PM UTC, comment #1: 

Yes, I can confirm this.  I've marked it as Minor with a priority of Later since most users will never encounter this.

I suppose there is something that is not being cleaned up during one of the tests.

Are you sure it is mappers.cc-tst that is at fault?  I copie that file into a local directory so I wouldn't have to run the whole test suite and then did 'test mappers.cc-tst' and didn't have any new windows pop up.

Rik <rik5>
Group administrator
Sat 17 Sep 2016 10:23:13 PM UTC, original submission:  

(Couldn't find if this was reported before, but I seem to have a vague reminiscence that it was mentioned somewhere a while ago)

When running __run_test_suite__.m in the GUI on Windows, several .m files get opened in the GUI editor around the time that mappers-cc.tst is tested. I.e., help.m, ls.m, dir.m, set.m and quantile.m, in that order.
I first noted this a few months ago. I suppose this is related to __run_test_suite__.m's operation.

I cannot reproduce this on Linux (that is, with __run_test_suite__.m in an installed Octave).

My mxe-octave setup is up-to-date and "standard".

Philip Nienhuis <philipnienhuis>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #44500:  bug49119.diff added by mtmiller (3KiB - text/x-patch)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by apjanke
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by dasergatskov (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by avinoam (Updated the item)
  • -email is unavailable- added by philipnienhuis (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 18 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-20 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2018-07-18 ttl StatusConfirmed Ready For Test
    2018-07-03 mtmiller Attached File- Added bug49119.diff, #44500
    2018-07-03 rik5 CategoryOctave Function GUI
        Severity2 - Minor 3 - Normal
        Priority1 - Later 5 - Normal
        Item GroupNone Other
        Carbon-Copy- Added ttl
    2018-06-29 rik5 Dependencies- bugs #54173 is dependent
    2018-06-25 apjanke Carbon-Copy- Added apjanke
    2016-09-21 mtmiller Summary_run_test_suite_ opens several files in GUI editor libinterp/corefcn/debug.cc-tst unit tests cause several files to open in GUI editor
    2016-09-20 dasergatskov Attached File- Added Screenshot_from_2016-09-20_10-27-33.png, #38564
    2016-09-19 rik5 Summary[MXE] _run_test_suite_ opens several files in GUI editor _run_test_suite_ opens several files in GUI editor
    2016-09-19 rik5 Operating SystemMicrosoft Windows Any
    2016-09-19 rik5 Severity3 - Normal 2 - Minor
        Priority5 - Normal 1 - Later
    2016-09-18 avinoam StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code