test(e2e): let a codex row name its second model once and add the no-comments rule

This commit is contained in:
ryan-crabbe-berri 2026-11-03 09:23:55 -08:00
parent d31dc43311
commit f22bdf3deb
4 changed files with 34 additions and 38 deletions

View file

@ -315,6 +315,8 @@ other.<area>.<case>.<assertion>
``` ```
## Hard Rules ## Hard Rules
- no comments in code unless they are truly necessary
- no unit tests of a product feature under `tests/e2e`, and no mock tests or monkeypatching of code anywhere in it: a product feature is proven end to end against a live proxy, never with a unit test. if a contributor asks you to write an end to end test, do NOT stage a unit test with it. If you find a product gap, call it out in the PR description. the harness's own plumbing is tested outside the suite, in `tests/e2e_harness/` (mirroring this folder's layout), because the Buildkite e2e run copies `tests/e2e/` into the runner image and runs every test in it, so a harness test in here would count as a product test in the nightly numbers. those tests run without a proxy and take their inputs as arguments or env vars (setting an env var through pytest's `monkeypatch` fixture is fine, patching a function, class, or module is not), and no coverage-registry or compat-matrix cell rests on them. judge a change inside one of them by that standard, not as a misplaced product test - no unit tests of a product feature under `tests/e2e`, and no mock tests or monkeypatching of code anywhere in it: a product feature is proven end to end against a live proxy, never with a unit test. if a contributor asks you to write an end to end test, do NOT stage a unit test with it. If you find a product gap, call it out in the PR description. the harness's own plumbing is tested outside the suite, in `tests/e2e_harness/` (mirroring this folder's layout), because the Buildkite e2e run copies `tests/e2e/` into the runner image and runs every test in it, so a harness test in here would count as a product test in the nightly numbers. those tests run without a proxy and take their inputs as arguments or env vars (setting an env var through pytest's `monkeypatch` fixture is fine, patching a function, class, or module is not), and no coverage-registry or compat-matrix cell rests on them. judge a change inside one of them by that standard, not as a misplaced product test
- use model management endpoints to create new models for a test. this could be in a conftest / inline for each test. ask the user what they want. - use model management endpoints to create new models for a test. this could be in a conftest / inline for each test. ask the user what they want.

View file

@ -97,21 +97,31 @@ def with_expected_failure(setup_id: SetupId, *, reason: str) -> tuple[ParameterS
) )
def also_driving(setups: tuple[ParameterSet, ...], extra: Mapping[str, Subject]) -> tuple[ParameterSet, ...]: @dataclass(frozen=True, slots=True)
class ExtraModel:
provider: Provider
litellm_params: LiteLLMParamsBody
def also_driving(setups: tuple[ParameterSet, ...], extra: Mapping[str, ExtraModel]) -> tuple[ParameterSet, ...]:
return tuple(_also_driving(setup, extra) for setup in setups) return tuple(_also_driving(setup, extra) for setup in setups)
def _also_driving(setup: ParameterSet, extra: Mapping[str, Subject]) -> ParameterSet: def _also_driving(setup: ParameterSet, extra: Mapping[str, ExtraModel]) -> ParameterSet:
deployment: Final = setup.values[0] deployment: Final = setup.values[0]
assert isinstance(deployment, Deployment) assert isinstance(deployment, Deployment)
added: Final = extra[deployment.litellm_params.model] added: Final = extra[deployment.litellm_params.model]
return pytest.param(*setup.values, id=setup.id, marks=tuple(_widened(mark, added) for mark in setup.marks)) return pytest.param(*setup.values, id=setup.id, marks=tuple(_widened(mark, added) for mark in setup.marks))
def _widened(mark: pytest.MarkDecorator | pytest.Mark, added: Subject) -> pytest.MarkDecorator | pytest.Mark: def _widened(mark: pytest.MarkDecorator | pytest.Mark, added: ExtraModel) -> pytest.MarkDecorator | pytest.Mark:
subject: Final = mark.args[0] if mark.name == "meta" and mark.args else None subject: Final = mark.args[0] if mark.name == "meta" and mark.args else None
if not isinstance(subject, Subject): if not isinstance(subject, Subject):
return mark return mark
return meta( return meta(
replace(subject, providers=(*subject.providers, *added.providers), models=(*subject.models, *added.models)) replace(
subject,
providers=(*subject.providers, added.provider),
models=(*subject.models, added.litellm_params.model),
)
) )

View file

@ -3,7 +3,6 @@
from __future__ import annotations from __future__ import annotations
from collections.abc import Callable from collections.abc import Callable
from dataclasses import dataclass
from types import MappingProxyType from types import MappingProxyType
from typing import Final from typing import Final
@ -17,44 +16,34 @@ from codex.setups import (
GPT_ON_BEDROCK_RUNTIME, GPT_ON_BEDROCK_RUNTIME,
GPT_ON_OPENAI, GPT_ON_OPENAI,
Deployment, Deployment,
ExtraModel,
ModelSetup, ModelSetup,
also_driving, also_driving,
with_expected_failure, with_expected_failure,
) )
from e2e_config import unique_marker from e2e_config import unique_marker
from e2e_metadata import Provider, Subject from e2e_metadata import Provider
from models import LiteLLMParamsBody from models import LiteLLMParamsBody
pytestmark = pytest.mark.e2e pytestmark = pytest.mark.e2e
OTHER_GPT_ON_BEDROCK_MANTLE: Final = "bedrock_mantle/openai.gpt-6-luna" TERRA_ON_OPENAI: Final = ExtraModel(
OTHER_GPT_ON_BEDROCK_RUNTIME: Final = "bedrock/us.openai.gpt-6-luna" Provider.OPENAI, LiteLLMParamsBody(model="openai/gpt-6.1-terra", api_key="os.environ/OPENAI_API_KEY")
OTHER_GPT_ON_OPENAI: Final = "openai/gpt-6.1-terra"
OTHER_CLAUDE_ON_BEDROCK: Final = "bedrock/us.anthropic.claude-haiku-5-5"
@dataclass(frozen=True, slots=True)
class SwitchTo:
provider: Provider
litellm_params: LiteLLMParamsBody
TO_OTHER_GPT_ON_OPENAI: Final = SwitchTo(
Provider.OPENAI, LiteLLMParamsBody(model=OTHER_GPT_ON_OPENAI, api_key="os.environ/OPENAI_API_KEY")
) )
SWITCH_TO: Final = MappingProxyType( SWITCH_TO: Final = MappingProxyType(
{ {
GPT_ON_BEDROCK_MANTLE: SwitchTo( GPT_ON_BEDROCK_MANTLE: ExtraModel(
Provider.BEDROCK_MANTLE, Provider.BEDROCK_MANTLE,
LiteLLMParamsBody(model=OTHER_GPT_ON_BEDROCK_MANTLE, aws_region_name=BEDROCK_REGION), LiteLLMParamsBody(model="bedrock_mantle/openai.gpt-6-luna", aws_region_name=BEDROCK_REGION),
), ),
GPT_ON_BEDROCK_RUNTIME: SwitchTo( GPT_ON_BEDROCK_RUNTIME: ExtraModel(
Provider.BEDROCK, LiteLLMParamsBody(model=OTHER_GPT_ON_BEDROCK_RUNTIME, aws_region_name=BEDROCK_REGION) Provider.BEDROCK, LiteLLMParamsBody(model="bedrock/us.openai.gpt-6-luna", aws_region_name=BEDROCK_REGION)
), ),
GPT_ON_AZURE_OPENAI: TO_OTHER_GPT_ON_OPENAI, GPT_ON_AZURE_OPENAI: TERRA_ON_OPENAI,
GPT_ON_OPENAI: TO_OTHER_GPT_ON_OPENAI, GPT_ON_OPENAI: TERRA_ON_OPENAI,
CLAUDE_ON_BEDROCK: SwitchTo( CLAUDE_ON_BEDROCK: ExtraModel(
Provider.BEDROCK, LiteLLMParamsBody(model=OTHER_CLAUDE_ON_BEDROCK, aws_region_name=BEDROCK_REGION) Provider.BEDROCK,
LiteLLMParamsBody(model="bedrock/us.anthropic.claude-haiku-5-5", aws_region_name=BEDROCK_REGION),
), ),
} }
) )
@ -78,12 +67,7 @@ def key_models(setup: ModelSetup, second_model: str) -> tuple[str, ...]:
reason="LiteLLM forwards the reasoning Azure encrypted to OpenAI, which cannot decrypt another account's" reason="LiteLLM forwards the reasoning Azure encrypted to OpenAI, which cannot decrypt another account's"
" reasoning and rejects the turn with invalid_encrypted_content (#41792)", " reasoning and rejects the turn with invalid_encrypted_content (#41792)",
), ),
MappingProxyType( SWITCH_TO,
{
model: Subject(providers=(to.provider,), models=(to.litellm_params.model,))
for model, to in SWITCH_TO.items()
}
),
), ),
) )
class TestCodexSwitchModel: class TestCodexSwitchModel:

View file

@ -4,8 +4,9 @@ from types import MappingProxyType
from typing import Final from typing import Final
from _pytest.mark.structures import ParameterSet from _pytest.mark.structures import ParameterSet
from codex.setups import SETUPS, Deployment, also_driving, with_expected_failure from codex.setups import SETUPS, Deployment, ExtraModel, also_driving, with_expected_failure
from e2e_metadata import Provider, Subject from e2e_metadata import Provider, Subject
from models import LiteLLMParamsBody
SECOND_MODEL: Final = "openai/second-model" SECOND_MODEL: Final = "openai/second-model"
@ -28,11 +29,10 @@ def _model(setup: ParameterSet) -> str:
def test_also_driving_declares_the_extra_model_on_every_setup_and_keeps_the_xfail() -> None: def test_also_driving_declares_the_extra_model_on_every_setup_and_keeps_the_xfail() -> None:
second: Final = ExtraModel(Provider.OPENAI, LiteLLMParamsBody(model=SECOND_MODEL))
setups: Final = also_driving( setups: Final = also_driving(
with_expected_failure("azure_openai", reason="known bug"), with_expected_failure("azure_openai", reason="known bug"),
MappingProxyType( MappingProxyType({_model(setup): second for setup in SETUPS}),
{_model(setup): Subject(providers=(Provider.OPENAI,), models=(SECOND_MODEL,)) for setup in SETUPS}
),
) )
assert tuple(setup.id for setup in setups) == tuple(setup.id for setup in SETUPS) assert tuple(setup.id for setup in setups) == tuple(setup.id for setup in SETUPS)