diff --git a/make_post_sell/models/__init__.py b/make_post_sell/models/__init__.py index 078b0f7..067b50a 100644 --- a/make_post_sell/models/__init__.py +++ b/make_post_sell/models/__init__.py @@ -38,7 +38,13 @@ configure_mappers() def get_engine(settings, prefix="sqlalchemy."): - engine = engine_from_config(settings, prefix) + # SQLite requires check_same_thread=False for multi-threaded access + # (e.g. background reforge threads sharing the engine's connection pool) + url = settings.get(f"{prefix}url", "") + kwargs = {} + if "sqlite" in url: + kwargs["connect_args"] = {"check_same_thread": False} + engine = engine_from_config(settings, prefix, **kwargs) if engine.url.get_backend_name() == "sqlite": @event.listens_for(engine, "connect") diff --git a/make_post_sell/models/shop.py b/make_post_sell/models/shop.py index 50af0a2..2c82433 100644 --- a/make_post_sell/models/shop.py +++ b/make_post_sell/models/shop.py @@ -781,6 +781,7 @@ def reforge_discovery_ring_async(shop_id, session_factory): t = threading.Thread(target=_reforge, daemon=True) t.start() + return t def is_shop_name_available(dbsession, name): diff --git a/make_post_sell/tests/test_functional.py b/make_post_sell/tests/test_functional.py index de9ca24..9226080 100644 --- a/make_post_sell/tests/test_functional.py +++ b/make_post_sell/tests/test_functional.py @@ -139,9 +139,15 @@ class UnauthenticatedFunctionalTests(FunctionalTests): ) -class AuthenticatedFunctionalTests(FunctionalTests): +class _AuthenticatedBase(FunctionalTests): + """Base class with authenticated setUp and helpers. No test methods. + + TestBeacon and TestAnalytics inherit from this instead of + AuthenticatedFunctionalTests so they don't duplicate 100+ inherited tests. + """ + def setUp(self): - super(AuthenticatedFunctionalTests, self).setUp() + super().setUp() self.shop1_params = { "name": "russell's shop", @@ -224,7 +230,6 @@ class AuthenticatedFunctionalTests(FunctionalTests): self.user2 = get_or_create_user_by_email(self.dbsession, "test2@example.com") def _clean_up_user(self, user): - # print("deleteing user {} from dbsession.".format(user.name)) self.dbsession.delete(user) def _clean_up_shop(self, shop): @@ -233,19 +238,13 @@ class AuthenticatedFunctionalTests(FunctionalTests): def _clean_up_stripe(self): """clean up remote Stripe API by removing Customer objects.""" customers = get_all_stripe_customer_objects(self.dbsession) - # print("cleaning remote Stripe API by removing {} Customer objets.".format(len(customers))) for customer in customers: customer.delete() def tearDown(self): """Clean up between tests.""" - - # clean up remote Stripe API by removing Customer objects. self._clean_up_stripe() - - # Parent tearDown will drop all tables, no need to manually delete users - # This avoids session conflicts when transaction state is inconsistent - super(AuthenticatedFunctionalTests, self).tearDown() + super().tearDown() def log_in_user(self, user_creds): # Set the email in the session before posting to the verification challenge @@ -258,9 +257,112 @@ class AuthenticatedFunctionalTests(FunctionalTests): # Attach csrf to class if needed res_csrf = self.testapp.get("/") - # self.csrf = res_csrf.form.fields["csrf_token"][0].value return res_login + def _create_shop_helper( + self, user_creds=None, shop_params=None, log_out_user=False + ): + """Helper method to create a shop and return it.""" + if user_creds is None: + user_creds = self.user1_creds + + if shop_params is None: + shop_params = self.shop1_params + + # log in with given user credentials. + self.log_in_user(user_creds) + + # create a new shop. + redirect_res = self.testapp.post("/s/new", shop_params) + if redirect_res.status_int == 302: + res = redirect_res.follow() + else: + # Handle case where shop creation doesn't redirect (e.g. validation errors) + res = redirect_res + res_body = res.body.decode() + if "Great work, you created a shop!" not in res_body: + self.fail( + f"Shop creation failed. Status: {res.status_int}, Body: {res_body[:500]}" + ) + + self.assertIn( + "Great work, you created a shop! You may continue to setup your shop or start posting products!", + res.body.decode(), + ) + + # query the new shop from database. + shop = get_shop_by_name(self.dbsession, shop_params["name"]) + + # prove that shop is not ready for Stripe. + self.assertFalse(shop.is_stripe_ready) + self.assertTrue(shop.is_stripe_not_ready) + + # add stripe keys to the newly created shop to make it ready. + stripe_params = { + "form_section": "stripe-settings", + "stripe_public_api_key": shop_params["stripe_public_api_key"], + "stripe_secret_api_key": shop_params["stripe_secret_api_key"], + "submit": "Save Settings", + } + res = self.testapp.post(f"/s/{shop.id}/settings", stripe_params) + # Follow redirect if present + if res.status_int == 302: + res = res.follow() + res_body = res.body.decode() + self.assertIn( + "You set the shop's stripe_public_api_key.", + res_body, + ) + self.assertIn( + "You set the shop's stripe_secret_api_key.", + res_body, + ) + + # refresh shop attributes from database. + self.dbsession.refresh(shop) + + # prove that shop is ready for Stripe. + self.assertTrue(shop.is_stripe_ready) + self.assertFalse(shop.is_stripe_not_ready) + + if log_out_user: + self.testapp.get("/log-out") + + return shop + + def _create_shop_and_product_for_comments(self): + """Helper: create a shop + product with comments enabled, stay logged in.""" + shop = self._create_shop_helper( + user_creds=self.user1_creds, shop_params=self.shop1_params + ) + + # Enable comments on the shop + self.testapp.post( + f"/s/{shop.id}/settings", + { + "form_section": "comment-settings", + "comments-enabled-checkbox": "on", + "submit": "Save Settings", + }, + status=302, + ) + + # Create a product + redirect_res = self.testapp.post( + f"/p/new?shop_id={shop.id}", self.product1_params + ) + res = redirect_res.follow() + self.assertIn("Great, next you may upload files.", res.body.decode()) + + # Fetch the product from DB + products = get_all_products(self.dbsession).all() + self.assertTrue(len(products) > 0) + product = products[0] + + return shop, product + + +class AuthenticatedFunctionalTests(_AuthenticatedBase): def test_prevent_verification_code_brute_force(self): """do not accept valid verification challenge code after failures.""" @@ -338,78 +440,6 @@ class AuthenticatedFunctionalTests(FunctionalTests): self.assertEqual(res.status_int, 200) self.assertIn(b"You must have a shop editor role", res.body) - def _create_shop_helper( - self, user_creds=None, shop_params=None, log_out_user=False - ): - """Helper method to create a shop and return it.""" - if user_creds is None: - user_creds = self.user1_creds - - if shop_params is None: - shop_params = self.shop1_params - - # log in with given user credentials. - self.log_in_user(user_creds) - - # create a new shop. - redirect_res = self.testapp.post("/s/new", shop_params) - if redirect_res.status_int == 302: - res = redirect_res.follow() - else: - # Handle case where shop creation doesn't redirect (e.g. validation errors) - res = redirect_res - res_body = res.body.decode() - if "Great work, you created a shop!" not in res_body: - self.fail( - f"Shop creation failed. Status: {res.status_int}, Body: {res_body[:500]}" - ) - - self.assertIn( - "Great work, you created a shop! You may continue to setup your shop or start posting products!", - res.body.decode(), - ) - - # query the new shop from database. - shop = get_shop_by_name(self.dbsession, shop_params["name"]) - - # prove that shop is not ready for Stripe. - self.assertFalse(shop.is_stripe_ready) - self.assertTrue(shop.is_stripe_not_ready) - - # add stripe keys to the newly created shop to make it ready. - # Add form_section to shop_params for the stripe settings - stripe_params = { - "form_section": "stripe-settings", - "stripe_public_api_key": shop_params["stripe_public_api_key"], - "stripe_secret_api_key": shop_params["stripe_secret_api_key"], - "submit": "Save Settings", - } - res = self.testapp.post(f"/s/{shop.id}/settings", stripe_params) - # Follow redirect if present - if res.status_int == 302: - res = res.follow() - res_body = res.body.decode() - self.assertIn( - "You set the shop's stripe_public_api_key.", - res_body, - ) - self.assertIn( - "You set the shop's stripe_secret_api_key.", - res_body, - ) - - # refresh shop attributes from database. - self.dbsession.refresh(shop) - - # prove that shop is ready for Stripe. - self.assertTrue(shop.is_stripe_ready) - self.assertFalse(shop.is_stripe_not_ready) - - if log_out_user: - self.testapp.get("/log-out") - - return shop - def test_create_new_shop(self): """Test creating a new shop.""" self._create_shop_helper() @@ -2273,37 +2303,6 @@ class AuthenticatedFunctionalTests(FunctionalTests): # ── AJAX comment submission (MPS-0) ────────────────────────────── - def _create_shop_and_product_for_comments(self): - """Helper: create a shop + product with comments enabled, stay logged in.""" - shop = self._create_shop_helper( - user_creds=self.user1_creds, shop_params=self.shop1_params - ) - - # Enable comments on the shop - self.testapp.post( - f"/s/{shop.id}/settings", - { - "form_section": "comment-settings", - "comments-enabled-checkbox": "on", - "submit": "Save Settings", - }, - status=302, - ) - - # Create a product - redirect_res = self.testapp.post( - f"/p/new?shop_id={shop.id}", self.product1_params - ) - res = redirect_res.follow() - self.assertIn("Great, next you may upload files.", res.body.decode()) - - # Fetch the product from DB - products = get_all_products(self.dbsession).all() - self.assertTrue(len(products) > 0) - product = products[0] - - return shop, product - @patch("smtplib.SMTP") def test_ajax_comment_returns_json(self, mock_smtp): """AJAX POST to /comments/new returns 201 JSON with comment data.""" @@ -3401,7 +3400,7 @@ class LazyCartFunctionalTests(FunctionalTests): self.assertEqual(200, res.status_int) -class TestBeacon(AuthenticatedFunctionalTests): +class TestBeacon(_AuthenticatedBase): """Functional tests for the /signals/beacon endpoint.""" def _setup_shop_and_product(self): @@ -3552,7 +3551,7 @@ class TestBeacon(AuthenticatedFunctionalTests): self.assertEqual(count2, 1) -class TestAnalytics(AuthenticatedFunctionalTests): +class TestAnalytics(_AuthenticatedBase): """Functional tests for the /s/{shop_id}/analytics page.""" def test_analytics_requires_editor(self): diff --git a/make_post_sell/tests/test_integration.py b/make_post_sell/tests/test_integration.py index 7bdbf17..b138965 100644 --- a/make_post_sell/tests/test_integration.py +++ b/make_post_sell/tests/test_integration.py @@ -3275,11 +3275,10 @@ class TestAsyncRingReforgeIntegration(DatabaseIntegrationTests): transaction.commit() from ..models.shop import reforge_discovery_ring_async - reforge_discovery_ring_async(shop_id, self.session_factory) + t = reforge_discovery_ring_async(shop_id, self.session_factory) # Wait for background thread to finish - import time - time.sleep(2) + t.join(timeout=10) # Re-query shop from DB to see committed ring self.dbsession = get_tm_session(self.session_factory, transaction.manager) @@ -3301,14 +3300,13 @@ class TestAsyncRingReforgeIntegration(DatabaseIntegrationTests): from ..models import shop as shop_module # Start the first reforge - reforge_discovery_ring_async(shop_id, self.session_factory) + t = reforge_discovery_ring_async(shop_id, self.session_factory) # Immediately trigger dirty bit reforge_discovery_ring_async(shop_id, self.session_factory) # Wait for both runs to finish - import time - time.sleep(3) + t.join(timeout=10) # Ring should exist self.dbsession = get_tm_session(self.session_factory, transaction.manager) @@ -3337,10 +3335,9 @@ class TestAsyncRingReforgeIntegration(DatabaseIntegrationTests): transaction.commit() from ..models.shop import reforge_discovery_ring_async - reforge_discovery_ring_async(shop_id, self.session_factory) + t = reforge_discovery_ring_async(shop_id, self.session_factory) - import time - time.sleep(2) + t.join(timeout=10) self.dbsession = get_tm_session(self.session_factory, transaction.manager) shop = self.dbsession.query(Shop).filter_by(name="ring-test-shop").one() diff --git a/make_post_sell/tests/test_models.py b/make_post_sell/tests/test_models.py index 608d870..46b6063 100644 --- a/make_post_sell/tests/test_models.py +++ b/make_post_sell/tests/test_models.py @@ -2612,7 +2612,7 @@ class TestAsyncDiscoveryRing(unittest.TestCase): "make_post_sell.models.shop.compute_discovery_ring", side_effect=slow_compute, ): - reforge_discovery_ring_async("shop123", mock_factory) + t = reforge_discovery_ring_async("shop123", mock_factory) # Thread should have started self.assertTrue(started.wait(timeout=5)) @@ -2621,8 +2621,7 @@ class TestAsyncDiscoveryRing(unittest.TestCase): proceed.set() # Wait for thread to complete - import time - time.sleep(0.1) + t.join(timeout=5) mock_session.commit.assert_called() @@ -2665,7 +2664,7 @@ class TestAsyncDiscoveryRing(unittest.TestCase): try: # First call starts the thread - reforge_discovery_ring_async("shop_dirty", mock_factory) + t = reforge_discovery_ring_async("shop_dirty", mock_factory) self.assertTrue(started.wait(timeout=5)) # Second call while running should set dirty bit @@ -2674,7 +2673,7 @@ class TestAsyncDiscoveryRing(unittest.TestCase): # Let the first run finish — it should loop and run again proceed.set() - time.sleep(1) + t.join(timeout=5) finally: patcher2.stop() patcher1.stop() @@ -2724,7 +2723,7 @@ class TestAsyncDiscoveryRing(unittest.TestCase): try: # First call starts thread - reforge_discovery_ring_async("shop_multi", mock_factory) + t = reforge_discovery_ring_async("shop_multi", mock_factory) self.assertTrue(started.wait(timeout=5)) # Rapid-fire 5 more calls — should all collapse to one dirty bit @@ -2735,7 +2734,7 @@ class TestAsyncDiscoveryRing(unittest.TestCase): self.assertTrue(shop_module._reforge_dirty.get("shop_multi")) proceed.set() - time.sleep(1) + t.join(timeout=5) finally: patcher2.stop() patcher1.stop() @@ -2780,13 +2779,13 @@ class TestAsyncDiscoveryRing(unittest.TestCase): mock_session.get.return_value = mock_shop try: - reforge_discovery_ring_async("shop_expire", mock_factory) + t = reforge_discovery_ring_async("shop_expire", mock_factory) self.assertTrue(started.wait(timeout=5)) # Trigger dirty bit reforge_discovery_ring_async("shop_expire", mock_factory) proceed.set() - time.sleep(1) + t.join(timeout=5) finally: patcher2.stop() patcher1.stop() @@ -2808,10 +2807,9 @@ class TestAsyncDiscoveryRing(unittest.TestCase): with mock.patch("make_post_sell.models.shop.SASession") as MockSession: MockSession.return_value = mock_session - reforge_discovery_ring_async("shop_crash", mock_factory) + t = reforge_discovery_ring_async("shop_crash", mock_factory) - import time - time.sleep(0.2) + t.join(timeout=5) # State should be cleaned up despite exception self.assertNotIn("shop_crash", shop_module._reforge_running) @@ -2832,10 +2830,9 @@ class TestAsyncDiscoveryRing(unittest.TestCase): with mock.patch("make_post_sell.models.shop.SASession") as MockSession: MockSession.return_value = mock_session - reforge_discovery_ring_async("shop_gone", mock_factory) + t = reforge_discovery_ring_async("shop_gone", mock_factory) - import time - time.sleep(0.2) + t.join(timeout=5) # Should not have tried to commit mock_session.commit.assert_not_called()