Stateful tests found by randomly

[miketheman]

List of test runs that expose a non-deterministic test result (failure) when run with a specific random seed (ordering).

To reproduce, use TESTARGS="--randomly-seed=<SEED> -vvv" make tests before adding to this list.

Seeds:

• 105155306, 1843708888, 19699634 - 7d32abd (#15469)
• 3227833760, 235656747 - 15d3495 (#15469)
• 2329742959 - 1f0aff8 (#15469)
• 3916416581, 3294924425, 1890731092 - test: fix sort order #15747
• 1697538841

---

Original issue:

To reproduce:

TESTARGS="--randomly-seed=105155306 -vvv" make tests T=tests/unit/api/test_simple.py::TestSimpleDetail

(The T parameter is the narrowest scoping of tests I found that still exhibited the issue to save on a full test run.)

Other seeds that fail the same test:

• 1843708888
• 19699634

FAILED tests/unit/api/test_simple.py::TestSimpleDetail::test_with_files_no_serial[text/html-None] - AssertionError: assert equals failed

Failure:

========================================================================= FAILURES =========================================================================
________________________________________________ TestSimpleDetail.test_with_files_no_serial[text/html-None] ________________________________________________

self = <tests.unit.api.test_simple.TestSimpleDetail object at 0xffffa39bc310>, db_request = <pyramid.testing.DummyRequest object at 0xffff9ae9f310>
content_type = 'text/html', renderer_override = None

    @pytest.mark.parametrize(
        "content_type,renderer_override",
        CONTENT_TYPE_PARAMS,
    )
    def test_with_files_no_serial(self, db_request, content_type, renderer_override):
        db_request.accept = content_type
        project = ProjectFactory.create()
        releases = ReleaseFactory.create_batch(3, project=project)
        release_versions = sorted([r.version for r in releases], key=parse)
        files = [
            FileFactory.create(release=r, filename=f"{project.name}-{r.version}.tar.gz")
            for r in releases
        ]
        # let's assert the result is ordered by string comparison of filename
        files = sorted(files, key=lambda key: key.filename)
        urls_iter = (f"/file/{f.filename}" for f in files)
        db_request.matchdict["name"] = project.normalized_name
        db_request.route_url = lambda *a, **kw: next(urls_iter)
        user = UserFactory.create()
        JournalEntryFactory.create(submitted_by=user)

>       assert simple.simple_detail(project, db_request) == {
            "meta": {"_last-serial": 0, "api-version": API_VERSION},
            "name": project.normalized_name,
            "versions": release_versions,
            "files": [
                {
                    "filename": f.filename,
                    "url": f"/file/{f.filename}",
                    "hashes": {"sha256": f.sha256_digest},
                    "requires-python": f.requires_python,
                    "yanked": False,
                    "size": f.size,
                    "upload-time": f.upload_time.isoformat() + "Z",
                    "data-dist-info-metadata": False,
                    "core-metadata": False,
                }
                for f in files
            ],
        }
E       AssertionError: assert equals failed
E         {                                                                      {
E           'files': [                                                             'files': [
E                                                                                    {
E                                                                                      'core-metadata': False,
E                                                                                      'data-dist-info-metadata': False,
E                                                                                      'filename': 'LWlHXDYZQIKh-10.0.tar.gz',
E                                                                                      'hashes': {
E                                                                                        'sha256': 'd3a2990d1fc4c91a16eeb880dbd04f37bf645fb5df36126179
E                                                                                b7dffb692a5fe2',
E                                                                                      },
E                                                                                      'requires-python': None,
E                                                                                      'size': 5823,
E                                                                                      'upload-time': '2017-03-01T12:55:14.698742Z',
E                                                                                      'url': '/file/LWlHXDYZQIKh-10.0.tar.gz',
E                                                                                      'yanked': False,
E                                                                                    },
E             {                                                                      {
E               'core-metadata': False,                                                'core-metadata': False,
E               'data-dist-info-metadata': False,                                      'data-dist-info-metadata': False,
E               'filename': 'LWlHXDYZQIKh-8.0.tar.gz',                                 'filename': 'LWlHXDYZQIKh-8.0.tar.gz',
E               'hashes': {                                                            'hashes': {
E                 'sha256': '7b791e0d2c30b12063115d379d6c588758a4147b41b2a3c509          'sha256': '7b791e0d2c30b12063115d379d6c588758a4147b41b2a3c509
E         a38b1c76d955cf',                                                       a38b1c76d955cf',
E               },                                                                     },
E               'requires-python': None,                                               'requires-python': None,
E               'size': 7891,                                                          'size': 7891,
E               'upload-time': '2009-02-12T05:59:26.341814Z',                          'upload-time': '2009-02-12T05:59:26.341814Z',
E               'url': '/file/LWlHXDYZQIKh-10.0.tar.gz',                               'url': '/file/LWlHXDYZQIKh-8.0.tar.gz',
E               'yanked': False,                                                       'yanked': False,
E             },                                                                     },
E             {                                                                      {
E               'core-metadata': False,                                                'core-metadata': False,
E               'data-dist-info-metadata': False,                                      'data-dist-info-metadata': False,
E         ---                                                                    ---
E                 'sha256': '8e4adef422c7671d4e50db09cc893d9e3cc77610429dd68cf8          'sha256': '8e4adef422c7671d4e50db09cc893d9e3cc77610429dd68cf8
E         2728f00c2783b8',                                                       2728f00c2783b8',
E               },                                                                     },
E               'requires-python': None,                                               'requires-python': None,
E               'size': 2104,                                                          'size': 2104,
E               'upload-time': '2019-03-22T08:14:52.990585Z',                          'upload-time': '2019-03-22T08:14:52.990585Z',
E               'url': '/file/LWlHXDYZQIKh-8.0.tar.gz',
E               'yanked': False,
E             },
E             {
E               'core-metadata': False,
E               'data-dist-info-metadata': False,
E               'filename': 'LWlHXDYZQIKh-10.0.tar.gz',
E               'hashes': {
E                 'sha256': 'd3a2990d1fc4c91a16eeb880dbd04f37bf645fb5df36126179
E         b7dffb692a5fe2',
E               },
E               'requires-python': None,
E               'size': 5823,
E               'upload-time': '2017-03-01T12:55:14.698742Z',
E               'url': '/file/LWlHXDYZQIKh-9.0.tar.gz',                                'url': '/file/LWlHXDYZQIKh-9.0.tar.gz',
E               'yanked': False,                                                       'yanked': False,
E             },                                                                     },
E           ],                                                                     ],
E           'meta': {'_last-serial': 0, 'api-version': '1.1'},                     'meta': {'_last-serial': 0, 'api-version': '1.1'},

tests/unit/api/test_simple.py:261: AssertionError

Source test here:

warehouse/tests/unit/api/test_simple.py

Lines 240 to 286 in 38ba926

	@pytest.mark.parametrize(
	"content_type,renderer_override",
	CONTENT_TYPE_PARAMS,
	)
	def test_with_files_no_serial(self, db_request, content_type, renderer_override):
	db_request.accept = content_type
	project = ProjectFactory.create()
	releases = ReleaseFactory.create_batch(3, project=project)
	release_versions = sorted([r.version for r in releases], key=parse)
	files = [
	FileFactory.create(release=r, filename=f"{project.name}-{r.version}.tar.gz")
	for r in releases
	]
	# let's assert the result is ordered by string comparison of filename
	files = sorted(files, key=lambda key: key.filename)
	urls_iter = (f"/file/{f.filename}" for f in files)
	db_request.matchdict["name"] = project.normalized_name
	db_request.route_url = lambda *a, **kw: next(urls_iter)
	user = UserFactory.create()
	JournalEntryFactory.create(submitted_by=user)
	assert simple.simple_detail(project, db_request) == {
	"meta": {"_last-serial": 0, "api-version": API_VERSION},
	"name": project.normalized_name,
	"versions": release_versions,
	"files": [
	{
	"filename": f.filename,
	"url": f"/file/{f.filename}",
	"hashes": {"sha256": f.sha256_digest},
	"requires-python": f.requires_python,
	"yanked": False,
	"size": f.size,
	"upload-time": f.upload_time.isoformat() + "Z",
	"data-dist-info-metadata": False,
	"core-metadata": False,
	}
	for f in files
	],
	}
	assert db_request.response.headers["X-PyPI-Last-Serial"] == "0"
	assert db_request.response.content_type == content_type
	_assert_has_cors_headers(db_request.response.headers)
	if renderer_override is not None:
	db_request.override_renderer == renderer_override

However it is likely something about the setup/teardown of other tests is mutating some state that's being left over.

---

P.S. There's a smattering of statements that don't actually assert, might be worth fixing once this test passes.

Example:

warehouse/tests/unit/api/test_simple.py

Lines 285 to 286 in 38ba926

	if renderer_override is not None:
	db_request.override_renderer == renderer_override

Note the lack of a leading assert.

[di]

Here's another stateful failure with --randomly-seed=3227833760: https://github.com/pypi/warehouse/actions/runs/7957829420/job/21721870412?pr=15364

[di]

Here's another one: --randomly-seed=2329742959:
https://github.com/pypi/warehouse/actions/runs/7971390556/job/21834554609?pr=15427

[di]

Another one with: --randomly-seed=235656747:
https://github.com/pypi/warehouse/actions/runs/8007534583/job/21871911912?pr=15444

[di]

Another one with: --randomly-seed=3916416581:
https://github.com/pypi/warehouse/actions/runs/8009314618/job/21878157012

[miketheman]

I've got a PR that fixes all (marked with 🚀) but the last one - 3916416581 it doesn't reproduce for me. @di can you reproduce that one?

[di]

I was able to reproduce it locally once, but not every time. It's a weird failure, I would expect Release.classifiers to always be ordered based on:

warehouse/warehouse/packaging/models.py

Lines 520 to 525 in 675794b

	_classifiers: Mapped[list[Classifier]] = orm.relationship(
	secondary="release_classifiers",
	order_by=Classifier.ordering,
	passive_deletes=True,
	)
	classifiers = association_proxy("_classifiers", "classifier")

But maybe Classifier.ordering is getting set wrong somehow?

[miketheman]

But maybe Classifier.ordering is getting set wrong somehow?

I looked into this, and I think the only time we actually set the value on this column is either during the release-level classifiers sync command, or explicitly in tests. Otherwise, there's no explicit order, so the order is implicit from whomever inserted it first.

I have a patch to have the ClassifierFactory create ordering based on insert sequence, but I don't want to add it to the mix until we either see more failures of this kind and can repro the failure.

diff --git a/tests/common/db/classifiers.py b/tests/common/db/classifiers.py
index 01118c8bd..5339fa7f8 100644
--- a/tests/common/db/classifiers.py
+++ b/tests/common/db/classifiers.py
@@ -10,6 +10,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

+import factory
+
 from warehouse.classifiers.models import Classifier

 from .base import WarehouseFactory
@@ -18,3 +20,5 @@ from .base import WarehouseFactory
 class ClassifierFactory(WarehouseFactory):
     class Meta:
         model = Classifier
+
+    ordering = factory.Sequence(lambda n: n)

[miketheman]

Alternately, instead of testing for an ordered list of classifiers in this test, we could compare the results as a set, so as to determine equivalent content for the resulting classifiers, as test_upload_succeeds_creates_release() is mainly looking to see that the posted data comes back correctly, and the order shouldn't matter to that test.

[di]

Well, Release.classifiers should always be an alphabetically ordered list. Perhaps order_by=Classifier.ordering is wrong, and that should be order_by=<something that alphabetizes the Classifier.classifier string>?

[miketheman]

This feels like #12701 again?

[di]

Hmm, since we're natsorting them, maybe Release.classifiers doesn't even need an ordering anymore?

[miketheman]

Well, format_classifiers is used in jinja templates only for the classifiers used on a given package. As we're seeing with different seedings of random, sort order can flip if unspecified.

Over here, we use the manually-sorted classifiers from the trove-classifiers source package to declare the order, which also feels a little weird:

warehouse/warehouse/views.py

Lines 359 to 364 in 68c1db9

	.order_by(
	expression.case(
	{c: i for i, c in enumerate(sorted_classifiers)},
	value=Classifier.classifier,
	)
	)

[di]

This one still seems to be happening, with --randomly-seed=3294924425: https://github.com/pypi/warehouse/actions/runs/8055152986/job/22002003778

[di]

Another: --randomly-seed=1697538841: https://github.com/pypi/warehouse/actions/runs/8378481333/job/22943316123