Files
mattermost/AGENTS.md
05a293c603 E2E/Test: Fix failing tests (#38981)
* playwright: allow ensureFeatureFlag to set multiple flags, remove skipIfFeatureFlagNotSet

ensureFeatureFlag now accepts either a single (flagName, value) pair or a
{flagName: value} map, so prerequisites like
'await pw.skipIfFeatureFlagNotSet("ChannelAttributes", true)' can be
combined with other flag prerequisites into a single server restart
instead of one restart per flag.

skipIfFeatureFlagNotSet is removed entirely (definition, export, and
fixture wiring): it only ever skipped a test when a flag was off rather
than turning it on, which silently left specs gated on a flag that
defaults off (e.g. DiscoverableChannels, ManagedChannelCategories)
permanently un-exercised in CI.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: migrate skipIfFeatureFlagNotSet call sites to ensureFeatureFlag

Mechanical rename of skipIfFeatureFlagNotSet(flag, true) to
ensureFeatureFlag(flag, true) across channel_attributes, global_attributes,
board_attributes, channel_perm_rules, and abac specs/helpers. Adjacent
single-flag calls within the same test are merged into one
ensureFeatureFlag({flagA: true, flagB: true}) call so the server restarts
once per test instead of once per flag.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: mark tests broken by real DiscoverableChannels/ManagedChannelCategories as known issues

Both DiscoverableChannels and ManagedChannelCategories default to off and
were never set in the testcontainers CI baseline, so every spec gated on
skipIfFeatureFlagNotSet(flag, true) for these flags silently self-skipped
and never actually ran. Migrating them to ensureFeatureFlag makes the
server restart with the flag genuinely on, which surfaces pre-existing
product bugs:

- discoverable_channels.spec.ts: Browse Channels' search results race the
  typed-search fetch against the discoverable-only loadDiscoverableChannels()
  fetch, intermittently dropping the 'Request to join' button. The sibling
  Find Channels flow is unaffected and still passes.
- managed_categories.spec.ts: assigning a managed_category_name to a
  channel has no effect on the sidebar in this server build - channels
  stay under the regular CHANNELS group and no managed category header is
  ever rendered, so every behavior that depends on being in a managed
  category (grouping, sorting, real-time updates, disabled Favorite/Move
  To/context-menu, and the System Console setting) fails.

Mark the affected tests with test.fixme and a @knownIssue comment so they
report as skipped rather than failing CI, while the two tests that don't
depend on the broken behavior keep running normally.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: force ChannelAttributes off in channel_classification.spec.ts

ChannelAttributes now defaults on (#38937), which hides the legacy
classification toggle this suite exercises (gated on
!channelAttributes.enabled in new_channel_modal.tsx and
channel_settings_configuration_tab.tsx). Force the flag off via
ensureFeatureFlag so the toggle renders again.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: force ChannelAttributes off in channel_banner.spec.ts

Same root cause as channel_classification.spec.ts: with ChannelAttributes
on by default, channel_settings_configuration_tab.tsx renders the
attribute-aware BannerTextEditor instead of the legacy AdvancedTextbox
(testid channel_banner_banner_text_textbox) this suite drives. Force the
flag off via ensureFeatureFlag so the legacy control renders again.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: give permalink preview wait more headroom in initial_scroll_read.spec.ts

The 'with multiple pages of post previews' case waits on a .post-preview
locator with Playwright's implicit 30s default timeout while 120
permalink previews resolve. Under full 30-way parallel CI load this
consistently took 39-44s (observed across two separate dispatch leases
in CI run 37112179268), timing out even though the behavior itself is
correct. Explicitly extend the wait to 60s and the per-test timeout to
90s for this case only.

Co-authored-by: saturnino <saturnino@mattermost.com>

* eslint-plugin: add playwright-ensure-feature-flag-top-level rule

Enforces that a Playwright spec file calls ensureFeatureFlag at most
once, and only from a top-level test.beforeAll(). This keeps the
intent of a spec's feature-flag combination explicit at the top of
the file instead of scattered through individual tests, and bounds
each spec file to at most one server restart.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: enable ensure-feature-flag-top-level rule, grandfather pre-existing files

Turns the new rule on as an error for the whole specs/ tree, and
carves out the 14 files that already called ensureFeatureFlag more
than once per file or outside a top-level beforeAll before this rule
existed. Those predate this change and are unrelated to it; each
needs its own migration pass to a single top-level beforeAll.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: hoist ensureFeatureFlag to a single top-level beforeAll in channel_attributes specs

Consolidates the per-test ensureFeatureFlag calls in
channel_attribute_assignment, channel_attribute_banner,
channel_attribute_banner_settings, channel_attribute_lifecycle, and
channel_attribute_settings_modal into one top-level beforeAll each,
per the new playwright-ensure-feature-flag-top-level rule.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: hoist ensureFeatureFlag to a single top-level beforeAll in channel_attribute_display/editing specs

Consolidates per-test ensureFeatureFlag calls into one top-level
beforeAll per file. The flag-off scenario in channel_attribute_display
needs the opposite ChannelAttributes value from every other test in
that file, so it moves to its own channel_attribute_display_flag_off.spec.ts.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: add channel_attribute_display_flag_off.spec.ts

Split out of channel_attribute_display.spec.ts: the flag-off scenario
needs the opposite ChannelAttributes value from every other test in
that file.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: mark 3 channel attribute chip-overflow tests as known issues

These three tests fail on master independent of this PR (reproduced
against the committed baseline via git stash, and live against a
Testcontainers server), each from a distinct bug in the
ChannelAttributeLabels/useLabelsOverflow chip-collapse feature:

- 'collapses the thread header chips into +N without moving the
  thread controls': availableWidthForLabels only measures the chip
  row's own flex ancestor, which in the thread pane has no visibility
  into how much room the separate Follow/popout/menu controls need.
- 'collapses overflowing chips into +N without moving header
  controls': channel-header__icons' deliberate min-width:0 lets it
  shrink below what its own non-shrinking icon buttons need, and the
  chip row is forced to show one chip even with zero room left,
  spilling ~12px past the header.
- 'adds and removes a multiselect option, and the header chips
  follow': removing one of two multiselect values leaves the chip
  container permanently visibility:hidden -- useLabelsOverflow's
  debounced recalculation after the ids change never completes.

Each is documented with a @knownIssue explaining the root cause found
via live reproduction, matching the convention already used for the
managed categories known issues.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: split managed_categories.spec.ts flag-off test into its own file

ManagedChannelCategories true/false needs two separate server
restarts, so the one test needing the flag off moves to
managed_categories_flag_off.spec.ts, leaving a single top-level
beforeAll(true) in managed_categories.spec.ts per the new
playwright-ensure-feature-flag-top-level rule.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: hoist ensureFeatureFlag to a single top-level beforeAll in misc specs

Consolidates per-test ensureFeatureFlag calls into one top-level
beforeAll each in channel_perm_rules_v0_4, discoverable_channels,
abac/policies/graph_operators, abac/resource_attributes/graph_sync,
and abac/resource_attributes/masking, per the new
playwright-ensure-feature-flag-top-level rule.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: split channel_attributes_required_kill_switch.spec.ts by flag value

ChannelAttributesRequired true/false needs two separate server
restarts, so the file splits into
channel_attributes_required_kill_switch_on.spec.ts and
channel_attributes_required_kill_switch_off.spec.ts, each with a
single top-level beforeAll, per the new
playwright-ensure-feature-flag-top-level rule.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: hoist ensureFeatureFlag to a single top-level beforeAll in global_attributes_form.spec.ts

Consolidates the per-test flag combinations (ChannelAttributes,
PostAttributes, PropertyFieldGraph) into one superset beforeAll: all
three are purely additive, so the superset covers every test here
with a single restart. Also stops requireHierarchicalAttributesEnabled
in global_attributes_helpers.ts from hiding an indirect
ensureFeatureFlag call of its own.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: hoist ensureFeatureFlag and split classification markings test

Adds a single top-level beforeAll(PropertyFieldGraph: true) to
global_attributes_listing.spec.ts, needed after
requireHierarchicalAttributesEnabled stopped setting the flag itself.
Splits the Classification Markings row test, which needs
ClassificationMarkings on, into its own
global_attributes_listing_classification_markings.spec.ts, per the
new playwright-ensure-feature-flag-top-level rule.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: hoist ensureFeatureFlag to a single top-level beforeAll in remaining global_attributes specs

Consolidates per-test ensureFeatureFlag calls into one top-level
beforeAll each in channel_resource_configuration.spec.ts and
classification_attribute.spec.ts, per the new
playwright-ensure-feature-flag-top-level rule.

Co-authored-by: saturnino <saturnino@mattermost.com>

* eslint-plugin: revert playwright-ensure-feature-flag-top-level rule

Drop the custom rule enforcing ensureFeatureFlag at the top level of a
spec file. A guideline in AGENTS.md and the function's own doc comment
is enough; tooling to enforce it is unnecessary overhead. Also reverts
eslint.config.mjs's enablement of the rule and its grandfather list.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: support multiple flags in ensureFeatureFlag, refresh client after restart

ensureFeatureFlag now also accepts a {flagName: value} map so a test
can combine several prerequisite flags into a single restart instead
of one per flag.

Also fixes a stale-client bug in its own post-restart verification:
restartMattermostContainer() points the server at a new container and
clears the cached client, so the pre-restart adminClient was stale by
the time it was used to verify the flags took effect. Fetch a fresh
client instead.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: call ensureFeatureFlag per test, remove skipIfNoEnterpriseLicense, fix stale adminClient reuse

Revert the top-level beforeAll/bare-import ensureFeatureFlag calls back
to per-test pw.ensureFeatureFlag() calls, since pw is a test-scoped
fixture and unavailable in beforeAll. Every test in a given spec file
passes the same parameters, so the Testcontainers server still restarts
at most once per file.

Replace the local skipIfNoEnterpriseLicense helper with
pw.skipIfNoLicense().

Fix several places where an adminClient obtained before
ensureFeatureFlag's restart was reused afterward: the restart points the
server at a new container, leaving that client stale. Reorder calls so
ensureFeatureFlag runs first, or refresh the client afterward when
reordering is not possible.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: restore ChannelAttributes=false override in channel_classification.spec.ts

Lost during the top-level-to-per-test ensureFeatureFlag revert. Without
it, ChannelAttributes defaulting on hides the legacy classification
toggle this suite exercises (gated on !channelAttributes.enabled in
new_channel_modal.tsx and channel_settings_configuration_tab.tsx),
failing every test with a timeout waiting for
channelClassificationToggle-button.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: give permalink preview wait more headroom in initial_scroll_read.spec.ts

120 permalink previews each resolve their own lookup, which can exceed
the default test timeout under loaded CI. Extend the timeout for that
case to leave headroom on top of the one-minute postPreview.waitFor.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: document ensureFeatureFlag convention and PW_WORKERS in AGENTS.md

Add e2e-tests/playwright/AGENTS.md covering the single-worker-per-server
parallelism model, the ensureFeatureFlag one-restart-per-file convention,
and pw.skipIfNoLicense(). Import it from the root AGENTS.md.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: stop custom profile attribute fields leaking across spec files

attribute_settings_behavior.spec.ts's MM-T5749 test created a custom
profile attribute field named "Cancel Test <id>" and never deleted it,
unlike its sibling spec files. Since CI runs many spec files against
one shared server per job, the field persisted and leaked into every
later test's Profile modal in that job.

Its Edit button's computed accessible name becomes "Cancel Test <id>
Edit" (title + button text via aria-labelledby), which
getByRole('button', {name: 'Cancel'})'s substring match also picked up
alongside the real Cancel button, causing a strict-mode violation in
locked_profile_fields.spec.ts whenever both specs ran in the same job
(reproduced locally by running them back-to-back on one server).

Fixes it two ways:
- Track and delete the fields attribute_settings_behavior.spec.ts owns,
  matching the deleteCustomProfileAttributes cleanup convention already
  used by its sibling spec files.
- Switch profile_modal.ts's cancelButton to getByTestId('cancelButton')
  instead of a substring role-name match, so this class of bug can't
  recur from any other attribute name.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: give long post-preview show-more check more headroom

The permalink preview needs a second render pass once the linked
post's content arrives before it knows whether to show the collapsed
'Show more' treatment. The default 10s expect timeout left no margin
for that extra round trip under loaded CI, causing this case alone
(out of 26 in the same file) to fail intermittently. Extend to 20s,
mirroring the same permalink-preview timing fix already applied to
initial_scroll_read.spec.ts.

Co-authored-by: saturnino <saturnino@mattermost.com>

* playwright: fix prettier formatting in attribute_settings_behavior.spec.ts

Co-authored-by: saturnino <saturnino@mattermost.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: saturnino <saturnino@mattermost.com>
2026-10-04 12:35:53 +08:00

742 B

AGENTS.md

Explicitly import subdirectory instruction files that must always be in context: @server/AGENTS.md @e2e-tests/playwright/AGENTS.md

Pull Requests

When creating a pull request, follow .github/PULL_REQUEST_TEMPLATE.md exactly:

  • Remove all <!-- --> comments.
  • Omit sections that are not applicable (Ticket Link, Screenshots) — do not write N/A, just remove the header.
  • The #### Release Note header and its "release-note" fenced code block **must always be present** (WITHOUT escaping the characters). Write NONE if the change has no API, schema, UI, or breaking changes.

Cursor Cloud Agents

Cursor Cloud Agents must follow .cursor/AGENTS.md, materialized by .cursor/scripts/cloud-agent-start.sh.