Opened 4 years ago

Closed 4 years ago

#21658 closed enhancement (fixed)

TestSuite: test that __new__ doesn't crash

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.5
Component: misc Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Marc Mezzarobba
Report Upstream: N/A Work issues:
Branch: 8140d6f (Commits) Commit: 8140d6fb5db8703178a4f2f0be427a8c89014810
Dependencies: #21662, #21663, #21664 Stopgaps:

Description (last modified by jdemeyer)

For any type t, the following should not crash Python:

t.__new__(t)

For Element classes, we also test

t.__new__(t, parent)

where parent is a parent (this mimics a typical call to __new__ for elements).

It is of course legal for this to raise ordinary exceptions.

One could argue that you should not call just __new__ like this since this will create invalid objects anyway. That is true, but this test checks for bugs in the object creation and deletion code. For example, if __new__ or __init__ or code calling __new__ raises a Python exception, then __dealloc__ should not segfault (which is exactly what caused the crash reported at #21628).

Bugs found: #21651, #21662, #21663, #21664

Change History (17)

comment:1 Changed 4 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from TestSuite(SageObject): test that __new__ doesn't crash Sage to TestSuite: test that __new__ doesn't crash

comment:2 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 4 years ago by jdemeyer

  • Dependencies set to #21662

comment:4 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/testsuite_sageobject___test_that___new___doesn_t_crash_sage

comment:5 Changed 4 years ago by jdemeyer

  • Commit set to 74d05c0c9c6dbc3b539b4d7ebd8c3d8e079e8c39
  • Dependencies changed from #21662 to #21662, #21663, #21664
  • Description modified (diff)

New commits:

74d05c0Test that __new__ doesn't crash

comment:6 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 4 years ago by git

  • Commit changed from 74d05c0c9c6dbc3b539b4d7ebd8c3d8e079e8c39 to d6eec0e9bccba14f4c90f8104ff2d32b2b498e19

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d6eec0eTest that __new__ doesn't crash

comment:9 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:11 Changed 4 years ago by git

  • Commit changed from d6eec0e9bccba14f4c90f8104ff2d32b2b498e19 to 3880b4839764c8559262cdc0a90467eb809e0cbc

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c0b3e39Fix initialization of ComplexIntervalFieldElement
2367f85Fix initialization of rational vectors
7323945Fix initialization of integer vectors
72d62c4Allow singular_ring_delete(NULL)
42c6423Merge remote-tracking branches 'trac/u/jdemeyer/fix_initialization_of_complexintervalfieldelement', 'trac/u/jdemeyer/fix_initialization_of_vectors' and 'trac/u/jdemeyer/allow_singular_ring_delete_null_' into t/21658/testsuite_sageobject___test_that___new___doesn_t_crash_sage
3880b48Test that __new__ doesn't crash

comment:12 Changed 4 years ago by jdemeyer

  • Status changed from new to needs_review

comment:13 Changed 4 years ago by mmezzarobba

  • Reviewers set to Marc Mezzarobba
  • Status changed from needs_review to positive_review

comment:14 Changed 4 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Test failures (every verbose run of TestSuite() needs to be patched).

comment:15 Changed 4 years ago by git

  • Commit changed from 3880b4839764c8559262cdc0a90467eb809e0cbc to 8140d6fb5db8703178a4f2f0be427a8c89014810

Branch pushed to git repo; I updated commit sha1. New commits:

425533aMerge tag '7.5.beta0' into t/21658/testsuite_sageobject___test_that___new___doesn_t_crash_sage
8140d6fDoctest fixes for _test_new() TestSuite test

comment:16 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-7.4 to sage-7.5
  • Status changed from needs_work to positive_review

I only added some obvious doctest fixes, so I take the liberty to set this back to positive_review myself. I did run make ptestlong.

comment:17 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/testsuite_sageobject___test_that___new___doesn_t_crash_sage to 8140d6fb5db8703178a4f2f0be427a8c89014810
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.