Tests: Over-mocking leads to errors surfaced in production

[miketheman]

One example:

warehouse/tests/unit/manage/views/test_oidc_publishers.py

Lines 1190 to 1348 in ee2fe41

	@pytest.mark.parametrize(
	("view_name", "publisher_form_obj", "expected_publisher"),
	[
	(
	"add_github_oidc_publisher",
	pretend.stub(
	validate=pretend.call_recorder(lambda: True),
	repository=pretend.stub(data="fakerepo"),
	normalized_owner="fakeowner",
	workflow_filename=pretend.stub(data="fakeworkflow.yml"),
	normalized_environment="some-environment",
	owner_id="1234",
	),
	pretend.stub(publisher_name="GitHub"),
	),
	(
	"add_gitlab_oidc_publisher",
	pretend.stub(
	validate=pretend.call_recorder(lambda: True),
	project=pretend.stub(data="fakerepo"),
	namespace=pretend.stub(data="fakeowner"),
	workflow_filepath=pretend.stub(data="subfolder/fakeworkflow.yml"),
	normalized_environment="some-environment",
	),
	pretend.stub(publisher_name="GitLab"),
	),
	(
	"add_google_oidc_publisher",
	pretend.stub(
	validate=pretend.call_recorder(lambda: True),
	email=pretend.stub(data="some-environment@example.com"),
	sub=pretend.stub(data="some-sub"),
	),
	"Google",
	),
	(
	"add_activestate_oidc_publisher",
	pretend.stub(
	validate=pretend.call_recorder(lambda: True),
	id="fakeid",
	publisher_name="ActiveState",
	publisher_url=lambda x=None: None,
	organization=pretend.stub(data="fake-org"),
	project=pretend.stub(data="fake-project"),
	actor=pretend.stub(data="fake-actor"),
	actor_id="some-user-id",
	),
	"ActiveState",
	),
	],
	)
	def test_add_oidc_publisher_created(
	self, metrics, monkeypatch, view_name, publisher_form_obj, expected_publisher
	):
	fakeuser = pretend.stub()
	project = pretend.stub(
	name="fakeproject",
	oidc_publishers=[],
	record_event=pretend.call_recorder(lambda *a, **kw: None),
	users=[fakeuser],
	)
	request = pretend.stub(
	user=pretend.stub(
	username="some-user",
	),
	registry=pretend.stub(
	settings={
	"github.token": "fake-api-token",
	}
	),
	find_service=lambda *a, **kw: metrics,
	flags=pretend.stub(
	disallow_oidc=pretend.call_recorder(lambda f=None: False)
	),
	session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
	POST=pretend.stub(),
	db=pretend.stub(
	query=lambda *a: pretend.stub(
	filter=lambda *a: pretend.stub(one_or_none=lambda: None)
	),
	add=pretend.call_recorder(lambda o: setattr(o, "id", "fakeid")),
	),
	path="request-path",
	)
	publisher_form_cls = pretend.call_recorder(lambda *a, **kw: publisher_form_obj)
	monkeypatch.setattr(oidc_views, "GitHubPublisherForm", publisher_form_cls)
	monkeypatch.setattr(oidc_views, "GitLabPublisherForm", publisher_form_cls)
	monkeypatch.setattr(oidc_views, "GooglePublisherForm", publisher_form_cls)
	monkeypatch.setattr(oidc_views, "ActiveStatePublisherForm", publisher_form_cls)
	monkeypatch.setattr(
	oidc_views,
	"send_trusted_publisher_added_email",
	pretend.call_recorder(lambda *a, **kw: None),
	)
	view = oidc_views.ManageOIDCPublisherViews(project, request)
	monkeypatch.setattr(
	view, "_hit_ratelimits", pretend.call_recorder(lambda: None)
	)
	monkeypatch.setattr(
	view, "_check_ratelimits", pretend.call_recorder(lambda: None)
	)
	assert isinstance(getattr(view, view_name)(), HTTPSeeOther)
	assert len(project.oidc_publishers) == 1
	publisher = project.oidc_publishers[0]
	assert view.metrics.increment.calls == [
	pretend.call(
	"warehouse.oidc.add_publisher.attempt",
	tags=[f"publisher:{publisher.publisher_name}"],
	),
	pretend.call(
	"warehouse.oidc.add_publisher.ok",
	tags=[f"publisher:{publisher.publisher_name}"],
	),
	]
	assert project.record_event.calls == [
	pretend.call(
	tag=EventTag.Project.OIDCPublisherAdded,
	request=request,
	additional={
	"publisher": publisher.publisher_name,
	"id": "fakeid",
	"specifier": str(publisher),
	"url": publisher.publisher_url(),
	"submitted_by": "some-user",
	"reified_from_pending_publisher": False,
	"constrained_from_existing_publisher": False,
	},
	)
	]
	assert request.session.flash.calls == [
	pretend.call(
	f"Added {str(publisher)} "
	+ (
	f"in {publisher.publisher_url()}"
	if publisher.publisher_url()
	else ""
	)
	+ " to fakeproject",
	queue="success",
	)
	]
	assert request.db.add.calls == [pretend.call(project.oidc_publishers[0])]
	assert publisher_form_obj.validate.calls == [pretend.call()]
	assert oidc_views.send_trusted_publisher_added_email.calls == [
	pretend.call(
	request,
	fakeuser,
	project_name="fakeproject",
	publisher=publisher,
	)
	]
	assert view._hit_ratelimits.calls == [pretend.call()]
	assert view._check_ratelimits.calls == [pretend.call()]

These tests don't exercise the stack down to the data layer, leading to issues discovered in production, like #18879, as well as noted in related changes here: #18862 (review)

The action here is to start with the trusted publishing tests, and refactor to use less pretend, and lean into using db_request and factory fixtures as necessary, replacing pretend in favor of real objects, with factory fixtures whenever possible.

[miketheman]

We have tests/functional that exercises about 48% of the overall code today.

However, a lot of those test still use fixtures and stubs, so converting them to use webtest-only style testing should happen first, so we can then evaluate what amount of the codebase is handled via webtesting.