Opened 8 years ago

Closed 5 years ago

#11638 closed enhancement (wontfix)

Patch: add integration unit tests

Reported by: mjo Owned by: burcin
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: symbolics Keywords:
Cc: Merged in:
Authors: Michael Orlitzky Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

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

Attachments (1)

add_known_integration_bug_tests.patch (1.0 KB) - added by mjo 8 years ago.

Download all attachments as: .zip

Change History (16)

Changed 8 years ago by mjo

comment:1 Changed 8 years ago by burcin

  • Status changed from new to needs_review

comment:2 Changed 8 years ago by kcrisman

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...

comment:3 follow-up: Changed 8 years ago by mjo

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.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 8 years ago by kcrisman

Replying to mjo:

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.

comment:5 in reply to: ↑ 4 Changed 8 years ago by mjo

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.

comment:6 Changed 8 years ago by mjo

  • Status changed from needs_review to needs_work

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

comment:7 Changed 8 years ago by mjo

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

comment:8 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:9 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:10 Changed 5 years ago by mjo

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.

comment:11 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:12 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:13 Changed 5 years ago by kcrisman

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix

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.

comment:14 Changed 5 years ago by mjo

  • Status changed from needs_work to positive_review

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.

comment:15 Changed 5 years ago by vbraun

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.