Skip to content

Commit 6aa208e

Browse files
committed
Sanitizing class method mock
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
1 parent 4237b6f commit 6aa208e

File tree

1 file changed

+37
-72
lines changed

1 file changed

+37
-72
lines changed

tests_openshift/database/test_tasks.py

Lines changed: 37 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,7 @@ def create_test_run_group(identifier: str):
487487
)
488488

489489

490+
@pytest.mark.xfail(reason="This test fails until the race condition of CoprBuildGroups is fixed")
490491
def test_copr_build_race_condition_concurrent_packages(
491492
clean_before_and_after,
492493
):
@@ -605,6 +606,42 @@ def patched_create_from_config_file(*args, **kwargs):
605606
# This ensures they execute concurrently and increases the chance of a race condition
606607
barrier = threading.Barrier(3, timeout=30)
607608

609+
def mock_create_with_new_run(*args, **kwargs):
610+
with sa_session_transaction(commit=True) as session:
611+
from sqlalchemy.orm import joinedload
612+
613+
srpm = session.query(SRPMBuildModel).filter_by(id=shared_srpm_id).first()
614+
pipeline = (
615+
session.query(PipelineModel)
616+
.options(
617+
joinedload(PipelineModel.copr_build_group),
618+
joinedload(PipelineModel.project_event),
619+
joinedload(PipelineModel.srpm_build),
620+
)
621+
.filter_by(id=shared_pipeline_id)
622+
.first()
623+
)
624+
625+
thread_id = threading.current_thread().ident
626+
pkg_name = thread_packages.get(thread_id, "unknown")
627+
pipeline.package_name = pkg_name
628+
session.add(pipeline)
629+
session.commit()
630+
631+
# Ensure relationships are loaded
632+
_ = pipeline.copr_build_group
633+
_ = pipeline.project_event
634+
_ = pipeline.srpm_build
635+
636+
session.expunge(srpm)
637+
session.expunge(pipeline)
638+
return srpm, pipeline
639+
640+
# Applying the mock to the class correctly
641+
flexmock(SRPMBuildModel).should_receive("create_with_new_run").replace_with(
642+
mock_create_with_new_run
643+
)
644+
608645
def run_copr_build_handler_for_package(package_name: str):
609646
"""Helper function to run copr_build_handler in a thread"""
610647
try:
@@ -842,78 +879,6 @@ def mock_get_valid_build_targets(self, *args, **kwargs):
842879
"/tmp/test.srpm"
843880
)
844881

845-
# CRITICAL: Mock SRPMBuildModel.create_with_new_run() to return the shared pipeline
846-
# This ensures all threads use the same pipeline, triggering the race condition
847-
# when they all try to create a CoprBuildGroup for it concurrently
848-
849-
from packit_service.models import SRPMBuildModel
850-
851-
def mock_create_with_new_run(cls, *args, **kwargs):
852-
# Query fresh objects from this thread's session to avoid SQLAlchemy
853-
# "already attached to session" errors. All threads will get objects
854-
# representing the same database row (same pipeline_id), which is what
855-
# we need to trigger the race condition.
856-
#
857-
# The race condition happens because:
858-
# 1. All threads query the same pipeline (same pipeline_id)
859-
# 2. Each thread checks run_model.copr_build_group (in-memory attribute)
860-
# 3. All see None (because they're checking their own session's object state)
861-
# 4. All try to set run_model.copr_build_group = build_group
862-
# 5. Only the last commit wins, causing the race condition
863-
#
864-
# The issue: CoprBuildGroupModel.create() checks run_model.copr_build_group
865-
# before merging the object into its session. If the object is detached,
866-
# this triggers a lazy load which fails. We need to ensure the relationship
867-
# is accessible without lazy loading. We'll use getattr with a default to
868-
# avoid triggering lazy loads, or we can ensure the object is properly merged.
869-
#
870-
# Actually, the best approach is to query the object fresh in each thread's
871-
# session, commit it (so it's persisted), then return it.
872-
# When CoprBuildGroupModel.create()
873-
# uses it, it will merge it into its own session. But we need to ensure the
874-
# relationship check doesn't trigger a lazy load.
875-
#
876-
# Solution: Query with the relationship already loaded (eager load), then
877-
# after commit and expunge, the relationship value is already in the object's
878-
# __dict__, so accessing it won't trigger a lazy load.
879-
with sa_session_transaction(commit=True) as session:
880-
# Query with eager loading to populate all relationships in __dict__
881-
# This prevents lazy load errors when the object is detached
882-
from sqlalchemy.orm import joinedload
883-
884-
shared_srpm = session.query(SRPMBuildModel).filter_by(id=shared_srpm_id).first()
885-
shared_pipeline = (
886-
session.query(PipelineModel)
887-
.options(
888-
joinedload(PipelineModel.copr_build_group),
889-
joinedload(PipelineModel.project_event),
890-
joinedload(PipelineModel.srpm_build),
891-
)
892-
.filter_by(id=shared_pipeline_id)
893-
.first()
894-
)
895-
# Update package_name to match this thread's package
896-
# (This simulates different packages trying to use the same pipeline)
897-
# Note: Since all threads update the same shared pipeline, the last one wins
898-
shared_pipeline.package_name = package_name
899-
session.add(shared_pipeline)
900-
session.commit()
901-
# Access all relationships to ensure they're loaded into __dict__
902-
# This prevents lazy load errors when the object is detached
903-
_ = shared_pipeline.copr_build_group # Load it now
904-
_ = shared_pipeline.project_event # Load it now (needed for cloning)
905-
_ = shared_pipeline.srpm_build # Load it now (needed for cloning)
906-
# Expunge to detach - but the relationship values are in __dict__
907-
session.expunge(shared_srpm)
908-
session.expunge(shared_pipeline)
909-
return shared_srpm, shared_pipeline
910-
911-
# Patch the classmethod directly (similar to
912-
# how CoprClient.create_from_config_file is patched)
913-
# create_with_new_run is a classmethod, so wrap it properly
914-
# Intentional patching for testing - ignore mypy errors
915-
SRPMBuildModel.create_with_new_run = classmethod(mock_create_with_new_run) # type: ignore
916-
917882
# Mock celery_app.send_task to prevent actual task sending
918883
from packit_service.celerizer import celery_app
919884

0 commit comments

Comments
 (0)