chore(tests): baseline-fixture app state instead of skip-on-error#231
Open
nijeesh-stream wants to merge 1 commit into
Open
chore(tests): baseline-fixture app state instead of skip-on-error#231nijeesh-stream wants to merge 1 commit into
nijeesh-stream wants to merge 1 commit into
Conversation
nijeesh-stream
force-pushed
the
nijeeshjoshy/chore-fix-shared-app-test-flakes
branch
4 times, most recently
from
9404b9f to
9128651
Compare
nijeesh-stream
changed the title
The shared CI test app accumulates blocklists from prior runs whose after-test delete didn't fire (CI abort, parallel run racing, etc). test_list_blocklists pins 'len(blocklists) == 3' (the two server defaults plus the 'Foo' fixture this test creates) and goes red as soon as that sediment piles up: 'expected: 3, got: 15'. The create-side of the contract is what this test actually exercises; the 'Foo' in names assertion already proves it. Loosen the count to >= 3 so a polluted app doesn't take down unrelated PRs while keeping the create signal. The .jpg upload and /giphy run_message_action tests on the same CI matrix also fail intermittently due to app-config drift (uploads MIME-type allowlist excluding image/jpeg, messaging channel-type missing the giphy command). Those need backend-side fixture work that's out of scope for this single chore PR — filing separately. Applied to both sync and async variants.
nijeesh-stream
force-pushed
the
nijeeshjoshy/chore-fix-shared-app-test-flakes
branch
from
9128651 to
c1218ca
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four integration tests routinely fail on CI for reasons unrelated to the code under test. All four assume specific app state on the shared test app; a prior PR's
update_app_settings/update_channel_typecan leave the app in a config that breaks unrelated tests later.Previous attempt added skip-on-error guards. That hid the real failure modes — if the app config ever drifts, all those tests become silent no-ops and we lose CI signal. The right fix is to pin the app state up front so the failure modes can't fire.
What this PR does
baseline_app_config— a module-scoped, autouse fixture in both sync and asyncconftest.py. Before any test in the module runs it:file_upload_configandimage_upload_configto empty allowlist + empty blocklist. Per the backendFileUploadConfig.Validateincontrollers/types.go, this means all extensions allowed. Unblockstest_send_and_delete_file.messagingchannel-type commands (giphy,imgur,flag,ban,mute,unban,unmute). Unblockstest_run_message_actionswhose/giphytext otherwise tripsunrecognized command(perserver/servercontext/types.go:324).test_send_and_delete_file— drop the skip-on-extension-error; baseline guarantees.jpgis accepted.test_run_message_actions— drop the skip-on-unrecognized-command; baseline guaranteesgiphyexists.test_list_blocklists— relax the exactlen(blocklists) == 3to>= 3+ keep the'Foo' in namescheck that proves the create-side of the contract. Shared app accumulates blocklists from prior runs whose delete didn't fire; pinning that to a per-app count from outside the test is a separate cleanup task.test_delete_channelsis intentionally left at the original 60-second poll. The asynq queue regularly taking >60s fordelete_channelsis a backend SLA bug and should be filed separately, not papered over with a 600-second bandage.Test plan
uv run black— clean