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

chai pollutes the global module namespace #18

Open
gpshead opened this Issue Apr 22, 2014 · 4 comments

Comments

Projects
None yet
2 participants
@gpshead

gpshead commented Apr 22, 2014

It's setUp() method pollutes the global namespace of any module using a TestCase inherited from Chai. It adds 'stub', 'expect', 'mock' and copies of all unittest.TestCase 'assert*' methods to the global module namespace from within its setUp() method.

It silently overwrites existing global names.

IMNSHO, this is a reason never to allow chai in your code base at all.

example: Have "from unittest import mock" in your test file? Sorry, you can't do that with chai. Chai overwrites that 'mock' with its own mock when setUp() is called.

@awestendorf

This comment has been minimized.

Show comment
Hide comment
@awestendorf

awestendorf Apr 22, 2014

Member

That's by design, which is why it's called out in the documents as a feature. I'm open to the idea of being able to turn that off with a class attribute.

Member

awestendorf commented Apr 22, 2014

That's by design, which is why it's called out in the documents as a feature. I'm open to the idea of being able to turn that off with a class attribute.

@gpshead

This comment has been minimized.

Show comment
Hide comment
@gpshead

gpshead Apr 23, 2014

A class attribute to turn that behavior off would be useful. The problem for me is that someone used Chai as a mixin within their own unittest.TestCase base class that they provide for others to use with functionality specific to their common needs. Other people then use it, and unexpected behavior happens within their code because they naturally know nothing of Chai as it isn't the common accepted unittest2 + mock way of writing unittests. With a class attribute, at least anyone who wants this behavior would be forced to explicitly declare their intent within their individual files if the base class they inherit from sets it to False.

I'd call the attribute "override_module_globals_with_chai" or something blindingly obvious to the chai-unfamiliar reader.

Another option that would alleviate some of the pain of integrating anything using chai into a larger codebase where chai is never going to be the favored kool-aid flavor used to write tests with is for it not to clobber any global names that already exist. (ie: you'd never overwrite 'mock'). This would mean tracking and undoing what you do in a tearDown (if not already done, i didn't look at that part of the code) to avoid names from one Chai TestCase from preventing a subsequent Chai test case from overwriting those since you are effectively shoving specific instance methods into the global namespace.

gpshead commented Apr 23, 2014

A class attribute to turn that behavior off would be useful. The problem for me is that someone used Chai as a mixin within their own unittest.TestCase base class that they provide for others to use with functionality specific to their common needs. Other people then use it, and unexpected behavior happens within their code because they naturally know nothing of Chai as it isn't the common accepted unittest2 + mock way of writing unittests. With a class attribute, at least anyone who wants this behavior would be forced to explicitly declare their intent within their individual files if the base class they inherit from sets it to False.

I'd call the attribute "override_module_globals_with_chai" or something blindingly obvious to the chai-unfamiliar reader.

Another option that would alleviate some of the pain of integrating anything using chai into a larger codebase where chai is never going to be the favored kool-aid flavor used to write tests with is for it not to clobber any global names that already exist. (ie: you'd never overwrite 'mock'). This would mean tracking and undoing what you do in a tearDown (if not already done, i didn't look at that part of the code) to avoid names from one Chai TestCase from preventing a subsequent Chai test case from overwriting those since you are effectively shoving specific instance methods into the global namespace.

@awestendorf

This comment has been minimized.

Show comment
Hide comment
@awestendorf

awestendorf Apr 23, 2014

Member

your last suggestion has long been on my list, I'll go with that approach

Member

awestendorf commented Apr 23, 2014

your last suggestion has long been on my list, I'll go with that approach

awestendorf added a commit that referenced this issue May 6, 2014

awestendorf added a commit that referenced this issue Jun 8, 2014

awestendorf added a commit that referenced this issue Jun 8, 2014

@awestendorf awestendorf referenced this issue Jun 8, 2014

Merged

Pep8 #20

awestendorf added a commit that referenced this issue Jun 8, 2014

awestendorf added a commit that referenced this issue Jun 8, 2014

@awestendorf

This comment has been minimized.

Show comment
Hide comment
@awestendorf

awestendorf Jun 9, 2014

Member

I pushed a fix out for this in 0.4.8. Sorry for the delay.

Member

awestendorf commented Jun 9, 2014

I pushed a fix out for this in 0.4.8. Sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment