engineering/make_post_sell
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases 2 Packages Wiki Activity Actions

fix: eliminate 173 duplicate inherited tests and harden async ring tests

Browse source 
Extract _AuthenticatedBase class from AuthenticatedFunctionalTests so
TestBeacon and TestAnalytics inherit only setUp/helpers without
duplicating all parent test methods (112 + 92 → 25 + 6 tests).

Add check_same_thread=False for SQLite engines so background reforge
threads can safely use the shared connection pool.

Return thread from reforge_discovery_ring_async and replace sleep-based
waits with thread.join() for deterministic, CI-resilient assertions.
This commit is contained in:
russell@unturf.com 2026-02-28 08:08:40 -05:00
parent 9d5d762b37
commit 136a3050fd
5 changed files with 141 additions and 141 deletions
Download patch file Download diff file Expand all files Collapse all files

8
make_post_sell/models/__init__.py
View file

@ -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")

1
make_post_sell/models/shop.py
View file

@ -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):

231
make_post_sell/tests/test_functional.py
View file

@ -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):

15
make_post_sell/tests/test_integration.py
View file

@ -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()

27
make_post_sell/tests/test_models.py
View file

@ -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()