Skip to content
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

TEST_CONFIG key has code using it but seems to be broken and no longer documented #2058

Open
fish-face opened this issue Nov 21, 2023 · 2 comments

Comments

@fish-face
Copy link

I saw this in documentation for 1.x without realising. Tried to use it and nothing changed - checked the code and there's still code for it but as far as I can tell nothing every calls make_test_backend. I suspect this is dead code. I did then dig a little more and it seems maybe you were supposed to use this with ChannelsLiveServerTestCase. As far as I can tell this still doesn't work; I can't use this TestCase directly (I need to inherit from another one) but I copied across the _pre_setup and _post_teardown methods just to make sure - but it's clear that they don't do anything to create a different channels backend in the current process, and I can't imagine there's anything happening in the spawned process either.

However, the original use case for TEST_CONFIG still exists: to avoid interfering with running dev instances. My plan was to have config like this:

CHANNEL_LAYERS = {
    'default': {
        'BACKEND': 'channels_redis.core.RedisChannelLayer',
        'CONFIG': {
            'hosts': [('redis', 6379)],
            'prefix': 'app:channels',
        },
        'TEST_CONFIG': {
            'hosts': [('redis', 6379)],
            'prefix': 'app:channels_test',
        },
    },
}

So that, similar to the test database, we have a completely different set of keys in redis - but don't have to run a separate redis instance.

Just to make sure, I ran MONITOR on the redis-cli to check that my hacky test case wasn't using the new prefix and indeed it was not - app:channels appears and not app:channels_test in the keys.

I think either the dead code ought to be removed or, preferably, the functionality to have a separate test config should be fixed or if in fact it does work and I'm just using it wrong, it ought to be documented again!

I will expand on a couple of points in case they're useful:

  1. we have no need to run these tests with a live server - if that was ever a limitation, it would be preferable to avoid it.
  2. we already need a custom subclass of TestCase to handle special setup and teardown (specifically for django-tenants if you're curious). There's an obvious advantage to bundling this functionality up in a TestCase subclass, but it makes sense to me to expose the functions such a subclass uses as its own API so that other subclasses can still make use of it cleanly and without lots of repeated code.
@carltongibson
Copy link
Member

Hi, thanks for the report, yes.

A quick grep, it looks like make_test_backend() is just never used.

I'm not sure what to say: I'd just use a different CHANNEL_LAYERS setting for tests, rather than branch on the same settings. This seems simpler… 🤔

So, I guess I'd lean towards removing it. Do you have an idea for making it work?

@fish-face
Copy link
Author

fish-face commented Nov 21, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants