pytest-django icon indicating copy to clipboard operation
pytest-django copied to clipboard

Prefix new fixtures with "django_"/"dj_"

Open blueyed opened this issue 9 years ago • 13 comments

IIRC we agreed to make fixture names more explicit / namespaced, e.g. django_mailoutbox. This should be done for new fixtures at least.

blueyed avatar Nov 21 '16 17:11 blueyed

Good idea... Alternative dj_mailoutbox, just a shorter version... This is something I have been thinking about when using some of the fixtures...

peterlauri avatar Nov 21 '16 21:11 peterlauri

Yes, a shorter prefix makes sense. So 👍 for dj_ (which would mean to rename/move some of the existing django_ ones, too).

blueyed avatar Nov 21 '16 21:11 blueyed

Maybe rename all public fixtures, and keep the old ones as aliases for a few releases?

On Mon, Nov 21, 2016 at 10:51 PM Daniel Hahler [email protected] wrote:

Yes, a shorter prefix makes sense. So 👍 for dj_.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pytest-dev/pytest-django/issues/432#issuecomment-262078951, or mute the thread https://github.com/notifications/unsubscribe-auth/ADWiQUHYv0tAWlH8ecYDwzS_1rHTyickks5rAhJ5gaJpZM4K4gdu .

peterlauri avatar Nov 21 '16 21:11 peterlauri

👍

We (@blueyed and me) discussed this during the pytest sprint. I forgot about it when reviewing the mailoutbox fixture. :/

Keeping aliases for quite some time and emit pytest warnings from them would be a good way to proceed. :)

pelme avatar Nov 23 '16 07:11 pelme

But related to dj_ vs django_: as you already have quite many fixtures named "correctly" with django_, maybe make sense to keep it? As well as markers...

peterlauri avatar Nov 23 '16 08:11 peterlauri

django_admin_client
django_admin_user
django_client
django_db
django_live_server
django_rf
django_settings
django_transactional_db
django_mailoutbox
...

vs

dj_admin_client
dj_admin_user
dj_client
dj_db
dj_live_server
dj_rf
dj_settings
dj_transactional_db
dj_mailoutbox

peterlauri avatar Nov 23 '16 08:11 peterlauri

I have two opinions:

  • To use dj_ as a prefix instead of django_. The text real estate in tests is valuable.

  • Old fixtures should be renamed to use dj_ as a prefix, by the way of a deprecation warning for a few versions

    This can be done via moving all fixtures to the prefix in the next version, and have the old fixtures function names wrap the new, except add a django warning for the user to switch to the new naming.

That way, in the next few versions, the old fixtures will be phased out completely, and everything will be consistent for users, and also easier to maintain on pytest-django side.

tony avatar Jan 18 '18 14:01 tony

The text real estate in tests is valuable.

Good point.

Let's go with dj_ then for new ones?!

blueyed avatar Jan 18 '18 17:01 blueyed

@blueyed I'm fine with that.

Does this mean to switch the new fixtures in #568 to use dj_?

tony avatar Jan 18 '18 18:01 tony

Yes, they should be changed.

However, I am not really confident: it means to change / migrate all existing fixtures - saving 4 characters per use of them only. The longer django_ prefix has the benefit of matching the plugin's name (after pytest-). So I would rather hear other opinions / feedback first.

blueyed avatar Jan 18 '18 18:01 blueyed

However, I am not really confident: it means to change / migrate all existing fixtures - saving 4 characters per use of them only. So I would rather hear other opinions / feedback first.

Okay, so I'll hold off updating #568 to dj_ for the moment. Or that could be merged in as-is if it's okay. Because if we begin migrating old fixtures to dj_, it'd end up being updated anyway. I favor an approach like mentioned in https://github.com/pytest-dev/pytest-django/issues/432#issuecomment-358665256 where a deprecation warning is temporarily given. The new fixtures in #568 wouldn't need a deprecation warning if merged as-is now unless there was a release published between now and transitioning the fixtures to (whichever) new prefix.

The longer django_ prefix has the benefit of matching the plugin's name (after pytest-).

That is an inconsistency and it's noted. I'm not a big fan of dj as a naming convention. But as a pytest prefix for fixtures, the compromise makes sense to me.

The reason I recommend dj_ is specifically due to the code and logic that gets scrunched into tests. Also, since pytest fixtures are in function arguments, the space their is also limited, in many cases, they end up becoming multiple lines very easily. The 4 characters are enough - but I only think it works if we were to tag a release where we did it all at once.

So that's my rationale/vote/opinion on it.

tony avatar Jan 18 '18 18:01 tony

Ref: https://github.com/pytest-dev/pytest/issues/4900.

I had a quick go on adding a "django_" prefix for all existing fixtures, while still providing the old name. Actually emitting a DeprecationWarning could be done later.

blueyed avatar Mar 13 '19 22:03 blueyed

@blueyed Was there any progress on this? I just accidentally overridden client that comes from pytest-django with a model factory created by pytest-factoryboy It took me some time to realize what is going on.

1oglop1 avatar Aug 31 '21 10:08 1oglop1