Skip to content

Commit 458dac8

Browse files
committed
fix: correct negotiator_ids extraction test logic
Tests now verify that only negotiators appearing in the trace are extracted, rather than asserting a fixed set. This correctly handles the case where a negotiation ends early (e.g., first offer is None) and fewer negotiators appear in the trace than were added to the mechanism.
1 parent 5724123 commit 458dac8

File tree

1 file changed

+25
-16
lines changed

1 file changed

+25
-16
lines changed

tests/core/test_common.py

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -875,39 +875,45 @@ def test_negotiator_ids_extracted_from_trace_format(self, tmp_path):
875875
"""Test negotiator_ids extraction works for 'trace' history type."""
876876
from negmas.mechanisms import CompletedRun
877877

878-
# Use p_ending=0.0 to ensure both negotiators appear in trace
878+
# RandomNegotiator may end negotiation early by offering None
879879
m = SAOMechanism(outcomes=[(i,) for i in range(10)], n_steps=10)
880-
m.add(RandomNegotiator(id="neg_a", name="A", p_ending=0.0))
881-
m.add(RandomNegotiator(id="neg_b", name="B", p_ending=0.0))
880+
m.add(RandomNegotiator(id="neg_a", name="A"))
881+
m.add(RandomNegotiator(id="neg_b", name="B"))
882882
m.run()
883883

884+
# Get the actual negotiators that appear in the trace
885+
actual_negotiators_in_trace = set(entry.negotiator for entry in m.full_trace)
886+
884887
# Save as single file with trace format
885888
completed = m.to_completed_run(source="trace")
886889
file_path = completed.save(tmp_path, "test_trace_format", single_file=True)
887890
loaded = CompletedRun.load(file_path)
888891

889-
# Should still extract negotiator_ids
892+
# Should extract only the negotiator_ids that appear in the trace
890893
assert "negotiator_ids" in loaded.config
891-
assert set(loaded.config["negotiator_ids"]) == {"neg_a", "neg_b"}
894+
assert set(loaded.config["negotiator_ids"]) == actual_negotiators_in_trace
892895

893896
def test_negotiator_ids_extracted_from_extended_trace_format(self, tmp_path):
894897
"""Test negotiator_ids extraction works for 'extended_trace' history type."""
895898
from negmas.mechanisms import CompletedRun
896899

897-
# Use p_ending=0.0 to ensure both negotiators appear in trace
900+
# RandomNegotiator may end negotiation early by offering None
898901
m = SAOMechanism(outcomes=[(i,) for i in range(10)], n_steps=10)
899-
m.add(RandomNegotiator(id="neg_x", name="X", p_ending=0.0))
900-
m.add(RandomNegotiator(id="neg_y", name="Y", p_ending=0.0))
902+
m.add(RandomNegotiator(id="neg_x", name="X"))
903+
m.add(RandomNegotiator(id="neg_y", name="Y"))
901904
m.run()
902905

906+
# Get the actual negotiators that appear in the trace
907+
actual_negotiators_in_trace = set(entry.negotiator for entry in m.full_trace)
908+
903909
# Save as single file with extended_trace format
904910
completed = m.to_completed_run(source="extended_trace")
905911
file_path = completed.save(tmp_path, "test_extended_trace", single_file=True)
906912
loaded = CompletedRun.load(file_path)
907913

908-
# Should still extract negotiator_ids
914+
# Should extract only the negotiator_ids that appear in the trace
909915
assert "negotiator_ids" in loaded.config
910-
assert set(loaded.config["negotiator_ids"]) == {"neg_x", "neg_y"}
916+
assert set(loaded.config["negotiator_ids"]) == actual_negotiators_in_trace
911917

912918
def test_negotiator_ids_not_extracted_from_history_format(self, tmp_path):
913919
"""Test that negotiator_ids are NOT extracted from 'history' format (no negotiator field)."""
@@ -933,14 +939,16 @@ def test_negotiator_ids_extracted_when_config_yaml_missing(self, tmp_path):
933939
from negmas.mechanisms import CompletedRun
934940
import os
935941

936-
# Use p_ending=0.0 to ensure RandomNegotiator never offers None
937-
# (which would end the negotiation early and potentially leave only
938-
# one negotiator in the trace)
942+
# RandomNegotiator may end negotiation early by offering None
939943
m = SAOMechanism(outcomes=[(i,) for i in range(10)], n_steps=10)
940-
m.add(RandomNegotiator(id="id_alpha", name="Alpha", p_ending=0.0))
941-
m.add(RandomNegotiator(id="id_beta", name="Beta", p_ending=0.0))
944+
m.add(RandomNegotiator(id="id_alpha", name="Alpha"))
945+
m.add(RandomNegotiator(id="id_beta", name="Beta"))
942946
m.run()
943947

948+
# Get the actual negotiators that appear in the trace
949+
actual_negotiators_in_trace = [entry.negotiator for entry in m.full_trace]
950+
unique_negotiators = list(dict.fromkeys(actual_negotiators_in_trace))
951+
944952
# Save as directory
945953
completed = m.to_completed_run(source="full_trace")
946954
dir_path = completed.save(tmp_path, "test_missing_config", single_file=False)
@@ -953,7 +961,8 @@ def test_negotiator_ids_extracted_when_config_yaml_missing(self, tmp_path):
953961
# Load and verify negotiator_ids are extracted from trace
954962
loaded = CompletedRun.load(dir_path)
955963
assert "negotiator_ids" in loaded.config
956-
assert set(loaded.config["negotiator_ids"]) == {"id_alpha", "id_beta"}
964+
# Should extract only the negotiators that appear in the trace
965+
assert loaded.config["negotiator_ids"] == unique_negotiators
957966
# negotiator_names should be same as IDs when extracted from trace
958967
assert loaded.config["negotiator_names"] == loaded.config["negotiator_ids"]
959968

0 commit comments

Comments
 (0)