bugGNU Octave - Bugs: bug #67237, "test pkg.tst" takes...

 
 

bug #67237: "test pkg.tst" takes long time

Submitter:  Dmitri A. Sergatskov <dasergatskov>
Submitted:  Tue 24 Jun 2025 06:43:34 PM UTC
   
 
Category:  Test Suite Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Performance
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Release: 
Operating System:  * Any Fixed Release:  11.1.0 (current default)
Planned Release:  11.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 10 Jul 2025 08:05:39 AM UTC, comment #14: 

@Rik: OK, no problem. Thank you.

Fernando <tutissanalio>
Thu 10 Jul 2025 07:22:27 AM UTC, comment #13: 

@Fernando: No offense intended.  The reasoning behind this was that it was a very small change (the addition of "./" to a string), almost like a suggestion on how to solve the problem.  And I did the actual work of preparing a changeset (changing the code, preparing the commit message referencing this bug, checking in the code to Mercurial with my login credentials).

For big additions to the code I always give credit to the original author.  If I have also worked on the patch significantly (whitespace changes and Octave code formatting are not significant) then I will add my name as well.  A good example of this is recent change to fix address sanitizer issues identified.


changeset:   34696:8591a26454e4
branch:      stable
tag:         tip
parent:      34692:8f3d778579f3
user:        Dmitri A. Sergatskov <dasergatskov@gmail.com> and Rik <rik@octave.org>
date:        Wed Jul 09 21:27:02 2025 +0200
summary:     Fix ASAN warnings in __ilu__.cc (bug #67249)


Dmitri made the original patch to fix the ASAN warnings, but I also made some important modifications (replacing C+98 usage of std::set::erase() and using std::swap to simplify code) so I added my name in addition to his as for the Author field.

I'll make sure going forward to ask you if you want to transform a potential patch into a full changeset.

Rik <rik5>
Group administrator
Thu 10 Jul 2025 07:02:45 AM UTC, comment #12: 

Not very important, but I am a little surprised to see that the modification proposed in comment #5 has been taken without giving any credit to its author.

Fernando <tutissanalio>
Wed 09 Jul 2025 12:54:38 PM UTC, comment #11: 

I made the small change to use a local, non-existent file to speed up BIST tests.  See https://hg.savannah.gnu.org/hgweb/octave/rev/a58794100b41.


Rik <rik5>
Group administrator
Tue 01 Jul 2025 04:33:43 AM UTC, comment #10: 

OK with me to close.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Tue 01 Jul 2025 02:18:40 AM UTC, comment #9: 

I think the simple change in comment #5 to change the BIST test to avoid connecting to the internet resolves the stated issue of this bug report ("takes long time").  Changes to pkg() function to detect already installed packages should be the subject of a new bug report.

Rik <rik5>
Group administrator
Thu 26 Jun 2025 03:02:41 PM UTC, comment #8: 

I see. A possibility would be to check if a package you are trying to install is already installed, and in that case either ask for confirmation or refuse to install (you can update it whith 'pkg update').

Fernando <tutissanalio>
Thu 26 Jun 2025 02:42:25 PM UTC, comment #7: 

This change (add "./") to the test is the absolute minimum that must be done here, but it is not my biggest gripe about "pkg".

I sometimes mis-typed "pkg install control", when I meant "pkg load control". Previously octave would correctly tell me that there is no file "control" to install and I'd correct myself and that is it. Now, it, instead, it happily goes to internet, grabs (broken) control package, wipes off my patched installed package and then shit itself trying to install the broken one. So now I have to go find my patched tarball install it etc... This is an utter disregard of the basic UI/UX principles.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 26 Jun 2025 02:04:27 PM UTC, comment #6: 

What I mean is that undesired connection to the internet to install a package can be easily avoided by specifying the package as a path (with at least one slash).

The same idea can be used in pkg.tst, if we do not want a test to connect to the internet when trying to install a non-existing local file.

Fernando <tutissanalio>
Wed 25 Jun 2025 02:38:52 PM UTC, comment #5: 

comentario nº3:

> Maybe, as a mitigating step, the look-up on the online package index should only be done for package names that do not contain dot "." characters.
> Would that be reasonable?
>


But dots seem to be valid characters in package names. Looking at get_pkg_info:


  ## Verify that name is valid.
  if (! (ischar (name) && rows (name) == 1 && ndims (name) == 2))
    error ("get_pkg_info: package NAME must be a string");
  elseif (! all (isalnum (name) | name == "-" | name == "." | name == "_"))
    error ("get_pkg_info: invalid package NAME: %s", name);
  endif


Another possibility would be to make this simple change:


diff -r da698895c3bc test/pkg/pkg.tst
--- a/test/pkg/pkg.tst  Tue Jun 24 16:17:35 2025 -0400
+++ b/test/pkg/pkg.tst  Wed Jun 25 16:34:45 2025 +0200
@@ -79,7 +79,7 @@
 %!   pkg ("uninstall", mfile_pkg_name{i});
 %! endfor
 %!
-%!error pkg ("install", "nonexistent.zip")
+%!error pkg ("install", "./nonexistent.zip")

 ## -local
 %!testif HAVE_Z


In that case, "./nonexistent.zip" is not a valid package name (because of the slash "/"), and Octave will not look for it online.

Fernando <tutissanalio>
Wed 25 Jun 2025 01:42:20 PM UTC, comment #4: 

I guess it would be better than what we have at the moment.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Wed 25 Jun 2025 01:25:18 PM UTC, comment #3: 

I'm also unsure whether it was a good idea to "randomly" connect to the internet without a dedicated flag for that. But here we are...

Maybe, as a mitigating step, the look-up on the online package index should only be done for package names that do not contain dot "." characters.
Would that be reasonable?

Markus Mützel <mmuetzel>
Group administrator
Wed 25 Jun 2025 12:56:53 PM UTC, comment #2: 

THe problem was when the "*.octave.org" were down (or slow),
it was trying to connect and timed out.
It is (reasonably) fast for me now.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Wed 25 Jun 2025 12:54:08 PM UTC, comment #1: 

I can't reproduce this with hg id da698895c3bc. Using Linux Mint 22.1 and octave built without installation (executed with run_octave).


octave:> tic, test pkg.tst, toc
PASSES 11 out of 11 tests
Elapsed time is 3.13858 seconds.
octave:> ver
----------------------------------------------------------------------
GNU Octave Version: 11.0.0 (hg id: da698895c3bc)
GNU Octave License: GNU General Public License
Operating System: Linux 6.8.0-62-generic #65-Ubuntu SMP PREEMPT_DYNAMIC Mon May 19 17:15:03 UTC 2025 x86_64
----------------------------------------------------------------------
no packages installed.


Fernando <tutissanalio>
Tue 24 Jun 2025 06:43:34 PM UTC, original submission:  

"test pkg.tst" takes about a minute. It hangs at "error pkg ("install", "nonexistent.zip")".
I think this is due to recent "enhancement" that "-forge" is implied (which I still think was wrong), so it tries to go to Internet and install "nonexixtent.zip" package from forge.

This should not happen.

As a minimum the test should be changed.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by tutissanalio (Posted a comment)
  • -email is unavailable- added by dasergatskov (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-07-09 rik5 Severity3 - Normal 2 - Minor
        Item GroupNone Performance
        StatusNone Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 11.1.0 (current default)
        CategoryOctave Function Test Suite
    2025-06-25 mmuetzel Planned ReleaseNone 11.1.0 (current default)

    Back to the top

    Powered by Savane 3.15-e6e5.
    Corresponding source code