Skip to content

Conversation

@scefali
Copy link
Contributor

@scefali scefali commented Mar 11, 2021

Previously, I had added DEMO_MODE=True in our pytest file so that all tests boot up in demo mode. This was because I needed the demo start route exposed and simply using @override_settings(DEMO_MODE=True) wasn't enough. Instead, I am making a separate URL file that demo mode will use and use @override_settings(DEMO_MODE=True, ROOT_URLCONF="sentry.demo.urls") to the routes in tests. This has the impact of making demo mode off by default which is preferable.

@scefali scefali requested a review from wedamija March 11, 2021 18:38
Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff, I like shrinking the demo footprint here


class DemoOrgManagerTeest(TestCase):
@override_settings(DEMO_ORG_OWNER_EMAIL=org_owner_email)
@override_settings(DEMO_MODE=True, DEMO_ORG_OWNER_EMAIL=org_owner_email)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can set @override_settings on a class as well. Does it make sense to make a DemoModeTestCase where we set this, then you don't need to set it on every test you write?

Could do the same thing for any views, have DemoModeViewTestCase and set the urlconf, although not sure how many views you're likely to have.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wedamija good call! Just made that change

@scefali scefali merged commit 7af703c into master Mar 11, 2021
@scefali scefali deleted the feat/demo-change-default-mode branch March 11, 2021 19:36
@scefali scefali mentioned this pull request Mar 15, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants