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

Patch: add integration unit tests #11638

Closed
orlitzky opened this issue Aug 1, 2011 · 10 comments
Closed

Patch: add integration unit tests #11638

orlitzky opened this issue Aug 1, 2011 · 10 comments

Comments

@orlitzky
Copy link
Contributor

orlitzky commented Aug 1, 2011

This adds unit tests for the following tickets: #11594, #11591, #11590, and #11238.

Component: symbolics

Author: Michael Orlitzky

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

@orlitzky orlitzky added this to the sage-5.11 milestone Aug 1, 2011
@burcin
Copy link

burcin commented Aug 1, 2011

comment:1

Attachment: add_known_integration_bug_tests.patch.gz

@kcrisman
Copy link
Member

comment:2

Hmm, I hadn't seen this before. Should we do this instead of adding them in piece by piece in some other file with each ticket? I do like it when we have record of things that have been fixed, but of course with lots maybe it is more convenient...

@orlitzky
Copy link
Contributor Author

comment:3

This was my first patch, so I just didn't know where to put things. The best thing to do would be get #12094 and #11483 reviewed so that more of these will work. Then we could stick them in the appropriate TESTS block.

Although, it is nice to have a collection of known bugs: you can always run them to see if a bug has been fixed by a package upgrade. If any have, it's trivial to copy/paste the doctest out and remove the optional flag.

@kcrisman
Copy link
Member

comment:4

Replying to @orlitzky:

This was my first patch, so I just didn't know where to put things. The best thing to do would be get #12094 and #11483 reviewed so that more of these will work. Then we could stick them in the appropriate TESTS block.

Does that mean #12094 is ready for review?

Although, it is nice to have a collection of known bugs: you can always run them to see if a bug has been fixed by a package upgrade. If any have, it's trivial to copy/paste the doctest out and remove the optional flag.

Not a bad point. We actually already have a similar file - look at calculus/wester.py.

@orlitzky
Copy link
Contributor Author

comment:5

Replying to @kcrisman:

Does that mean #12094 is ready for review?

Yes! Fixed. The subtleties of trac had not yet had time to sink in when I made that patch. There's no option to open a ticket as anything other than new, so it never occurred to me.

@orlitzky
Copy link
Contributor Author

comment:6

Bookkeeping: The doctest for #11594 went in as part of #11483.

@orlitzky
Copy link
Contributor Author

comment:7

And I just posted the doctest for #11591 to its ticket.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@orlitzky
Copy link
Contributor Author

comment:10

Random update: The test for #11594 is no longer needed, it went in another ticket.

#11591 test case still fails.

#11590 test case still fails.

#11238 no longer needed, it went in with the fix.

Just keeping track of known failures so that they can be checked from time to time.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@kcrisman
Copy link
Member

kcrisman commented Dec 8, 2014

comment:13

Hi! This is a good idea in principle, but I have a feeling that keeping it updated would be pretty challenging. By the way, #11591 seems to have worked for quite some time, but the doctest fails because you got the wrong parenthesization - 1/3*pi^2 versus pi^(2/3). So even though we have plenty of integral errors, this may not be a super-helpful way to do it, given our "manpower" resources.

What do you think? I propose wontfix, but I'm open to other ideas.

@kcrisman kcrisman removed this from the sage-6.4 milestone Dec 8, 2014
@orlitzky
Copy link
Contributor Author

orlitzky commented Dec 8, 2014

comment:14

Yeah that's fine. The idea was that we'd be notified if any of these were fixed upstream (since the tests would start failing). It makes just as much sense to open tickets for each failing integral and check them from time to time.

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

5 participants