Errors cause nosetests to be unable to continue, due to contract redefinition

[elistevens]

Consider a file foo.py (note, this is condensed from the real test case; apologies for typos, etc.):

from contracts import contract, new_contract

@new_contract
def myContract(inst):
    return isinstance(inst, object)

class Foo(object):
    @contract
    def __init__(self, a):
        """
        :type a: number,$$%^^&
        """
        pass

If foo gets imported in multiple test cases (we're running them via python setup.py nosetests), then the following occurs:

======================================================================
ERROR: Failure: ContractSyntaxError (Expected "(" (at char 8), (line:1, col:9)

 line  1 >number,$$%^^&
                  ^
                  |
                  here or nearby)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/nose/loader.py", line 390, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/local/lib/python2.7/dist-packages/nose/importer.py", line 39, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/local/lib/python2.7/dist-packages/nose/importer.py", line 86, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
...
  File "/usr/local/lib/python2.7/dist-packages/contracts/main.py", line 159, in contract_decorator
    return contracts_decorate(function, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/contracts/main.py", line 224, in contracts_decorate
    for x in accepts_dict])
  File "/usr/local/lib/python2.7/dist-packages/contracts/main.py", line 306, in parse_flexible_spec
    return parse_contract_string(spec)
  File "/usr/local/lib/python2.7/dist-packages/contracts/main.py", line 80, in parse_contract_string
    raise ContractSyntaxError(msg, where=where)
ContractSyntaxError: Expected "(" (at char 8), (line:1, col:9)

 line  1 >number,$$%^^&
                  ^
                  |
                  here or nearby

======================================================================
ERROR: Failure: ValueError (Tried to redefine 'myContract' with a definition that looks different to me.
 - old: CheckCallable(<function myContract at 0x3198410>)
 - new: CheckCallable(<function myContract at 0x39427d0>)
)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/nose/loader.py", line 390, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/local/lib/python2.7/dist-packages/nose/importer.py", line 39, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/local/lib/python2.7/dist-packages/nose/importer.py", line 86, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
...
  File "/usr/local/lib/python2.7/dist-packages/contracts/main.py", line 493, in new_contract
    new_contract_impl(identifier, function)
  File "/usr/local/lib/python2.7/dist-packages/contracts/main.py", line 577, in new_contract_impl
    raise ValueError(msg)
ValueError: Tried to redefine 'myContract' with a definition that looks different to me.
 - old: CheckCallable(<function myContract at 0x3198410>)
 - new: CheckCallable(<function myContract at 0x39427d0>)

The ValueError is repeated for every subsequent test case that imports foo, even if the class with the broken definition is not used. This can make it difficult to find the real error when there are a large number of test cases.

[AndreaCensi]

Just to see if I understand the problem, if you remove the contract in the docstring, then also the "Tried to redefine 'myContract' " error goes away, right?

If so, I think what's happening is that Nose keeps trying to import the module again even if it fails the first time. Every time, a "new" function "myContract" is instantiated. What do you think should happen at this point?

[elistevens]

Removing the syntax error in the docstring causes all of the errors to go away. I think that having a decorated function with no contract causes a different error (ContractException vs. ContractSyntaxError), but I haven't tested that directly.

And yes, I think that reimportation is what's causing the issue. As for what should happen... Maybe something like this?

# https://github.com/AndreaCensi/contracts/blob/master/src/contracts/main.py#L572
        if not(inspect.getsource(contract) == inspect.getsource(old)):
            msg = '...'

Not sure if a textual representation of the source is good enough or not. Redefining a contract function that uses a closure might cause a false negative, but that seems like it's pretty deep into "you should know better" territory to me. I think that this approach would make this problem go away. For me, that's a good tradeoff, but I haven't thought about the problem much. ;) I totally understand if you decide otherwise. :)

[AndreaCensi]

I think that perhaps the best way to check this is using the function name ("my.module.myfunction").

I'm not sure though I can implement a robust way to do that check, that works for all callable things (lambdas, functions from C extensions, etc.).

[elistevens]

What does the module.funcname get you over using inspect.getsource?

Although I should note that inspect.getsource doesn't work on lambda either. Maybe something like this:

import inspect
def functionId(f):
    try:
        return inspect.getsource(f)
    except IOError:
        return id(f)

And then use that:

if not(functionId(contract) == functionId(old)):

[AndreaCensi]

What does the module.funcname get you over using inspect.getsource?

I was thinking it would be better as it would not involve any inspection. Perhaps id is the best way overall. But I don't think it would work in your original example, where the module is only partially initialized. In that case, probably id() gives a different id to the two incarnations of the same function.

[elistevens]

Yeah, the id of the two functions is different. Note the "at 0x..." parts below:

ValueError: Tried to redefine 'myContract' with a definition that looks different to me.
 - old: CheckCallable(<function myContract at 0x3198410>)
 - new: CheckCallable(<function myContract at 0x39427d0>)

I suspect that function objects don't define equality differently than object (ie. it uses "is" to determine equality).

There is also the possibility of using the .func_code object; it's possible that code objects have a richer definition of equality. Not sure. See:

http://docs.python.org/reference/datamodel.html#types

[AndreaCensi]

I believe this was fixed some time ago.