Illegal use of locals in test suite

Issue #1073 resolved
Anonymous created an issue

According to python specification:

"""locals( )

Update and return a dictionary representing the current local symbol table. Warning: The contents of this dictionary should not be modified; changes may not affect the values of local variables used by the interpreter. """

line 66 of test/orm/ stands:

locals()42 = 99

Which is a bit strange and might not work on all python interpreters. What about changing it a bit more explicitely (like explicitely call setattr on a class, or modify a dict) to avoid such issues?

cheers, fijal

Comments (19)

  1. Anonymous

    It would not be a big deal (test will simply fail), but the problem is it happens on import level which prevents tests from running. On the other hand, I would like not to allow keeping strange stuff in dict of new-style classes in pypy (old style are fine). I would hope that relying on such detail (undocumented and untested) should not prevent pypy from running sqlalchemy.

    Cheers, fijal

  2. Anonymous
    • removed status
    • changed status to open

    According to:

    keeping non-string keys in type dictionaries is illegal. I'm reopening the ticket, but on the other hand, it should not be considered very crucial (even for running sqlalchemy on top of pypy), since this use case is only in tests. (Although this tests something for a reason, exactly one for which pypy will fail at some point further). I'm up to discussion whether this should be fixed or won't.

    Cheers, fijal

  3. Michael Bayer repo owner

    "fixing" this ticket as you say would then result in a new bug report "no test coverage for full AddOns compatibility", which is a more severe issue IMO. So I don't see what it accomplishes to reopen this ticket.

  4. jek

    i think we're agreed now that the current setup is ok: the line only runs under cpython, and does nothing other than ensure that cpython sqla's instrumentation doesn't explode when probing classes produced by AddOns, an unrelated cpython module from pypi.

  5. Anonymous

    ekhem Sorry to reopen this again, but this try: except: does not solve anything. This locals() substitution works, but creating a class afterwards doesn't.

  6. Michael Bayer repo owner

    Since I don't have access to pypy could you be more specific about what the exact behavior is ? I tried artificially placing an exception throw within the try/except block (before or after the locals() hack) and the test still completes successfully.

  7. jek

    my tests on pypy show that modifying locals() seems to work just fine, so the except: is never triggered. it croaks at the end of the class definition, presumably while assembling the locals dict into the class dictionary.

  8. Log in to comment