Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase harcoded timeouts default values #14179

Closed
jpflori opened this issue Feb 25, 2013 · 21 comments
Closed

Increase harcoded timeouts default values #14179

jpflori opened this issue Feb 25, 2013 · 21 comments

Comments

@jpflori
Copy link

jpflori commented Feb 25, 2013

Two hardcoded timeouts default valued make some tests fail on Cygwin (presumably because Cygwin is slow, and forking is horribly slow):

  • sage/tests/cmdline.py, the timeout value (current default 50.0 secs) of test_executable;
  • sagenb/misc/misc.py, the alarm value (current default 1 sec) when launcing the notebook (which makes lauching the notebook tricky.
    See #6743 comment:144 and #6743 comment:146

Just slightly (from a human point of view) increasing these values let the tests pass on recent and even not so recent and powerful harware running Cygwin, so let's increase these default values unconditionnally rather than adding Cygwin specific code, or warnings about expected to fail tests or whatever; let's say:

  • 100.0 secs in test_executable,
  • 5 secs when launching the notebook.

Apply:

Upstream: Fixed upstream, in a later stable release.

CC: @jdemeyer @kcrisman @dimpase

Component: porting: Cygwin

Keywords: cygwin timeout

Author: Jean-Pierre Flori

Reviewer: Jeroen Demeyer

Merged: sage-5.9.beta4

Issue created by migration from https://trac.sagemath.org/ticket/14179

@jpflori jpflori added this to the sage-5.9 milestone Feb 25, 2013
@jpflori
Copy link
Author

jpflori commented Feb 25, 2013

comment:1

Here comes a patch for the Sage library part.
Not sure of the procedure for modifying the notebook, I'll post a link to here on sage-notebook group.

@jpflori
Copy link
Author

jpflori commented Feb 25, 2013

comment:2

(Note the patch name says 14719, but fortunately the commit message states the correct ticket number, so I won't upload another patch, unless modifications are needed of course.)

@kini
Copy link
Collaborator

kini commented Feb 27, 2013

comment:3

For sagenb: sagemath/sagenb#137

@jpflori
Copy link
Author

jpflori commented Feb 27, 2013

comment:4

Replying to @kini:

For sagenb: sagemath/sagenb#137

Great, thanx!
We now just have to make this ticket depend on the future sagenb upgrade.

@kcrisman
Copy link
Member

kcrisman commented Apr 2, 2013

Upstream: Fixed upstream, in a later stable release.

@kcrisman
Copy link
Member

kcrisman commented Apr 2, 2013

comment:5

Patchbot, apply trac_14719-sage_library.patch

@jpflori
Copy link
Author

jpflori commented Apr 2, 2013

comment:6

The notebook fix is in version 0.10.5 and upgrading to this version is #14330.

@jpflori

This comment has been minimized.

@jpflori
Copy link
Author

jpflori commented Apr 2, 2013

Author: Jean-Pierre Flori

@jpflori
Copy link
Author

jpflori commented Apr 2, 2013

Dependencies: #14330

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2013

Changed dependencies from #14330 to none

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2013

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2013

comment:7

This doesn't actually depend on #14330.

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2013

Work Issues: rebase

@jdemeyer
Copy link

jdemeyer commented Apr 2, 2013

comment:8

And it needs to be rebased to #12415.

@jpflori
Copy link
Author

jpflori commented Apr 2, 2013

Changed work issues from rebase to none

@jpflori
Copy link
Author

jpflori commented Apr 2, 2013

comment:9

Attachment: trac_14179-sage_library.patch.gz

@jpflori

This comment has been minimized.

@jpflori
Copy link
Author

jpflori commented Apr 2, 2013

comment:10

In fact I'm not sure what needed to be rebased.
The patch I just uploaded was produced on top of 5.9.beta2 but looks exactly like the previous one.
So I'll think a little more.

@jpflori
Copy link
Author

jpflori commented Apr 2, 2013

comment:11

Oh, is that the 'r' before '"""'...

@jdemeyer
Copy link

jdemeyer commented Apr 4, 2013

Merged: sage-5.9.beta4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants