Should we ban "crash" in test name other than use that for test type hint?

[WeizhongX]

Looks like developers could get confused by naming convention for crash tests, e.g. chromium bug 379764806. Should we do something about that, e.g. ban "crash" in file name other than use that as a test type hint?

Below is a list of tests that have "crash" in test name but are not crash tests. Given ppl could get confused, there is really a question are they intended to be a crash test or not?

./css/css-break/break-inside-avoid-multicol-iframe-crash-print.html
./css/css-position/position-absolute-crash-chrome-012.html
./css/css-position/position-absolute-crash-chrome-002.html
./css/css-position/position-absolute-crash-chrome-003.html
./css/css-position/position-absolute-crash-chrome-011.html
./css/css-position/position-absolute-crash-chrome-006.html
./css/css-position/position-absolute-crash-chrome-013.html
./css/css-position/position-absolute-crash-chrome-001.html
./css/css-position/position-absolute-crash-chrome-010.html
./css/css-position/position-absolute-crash-chrome-009.html
./css/css-position/position-absolute-crash-chrome-005.html
./css/css-position/position-absolute-crash-chrome-007.html
./css/css-position/position-absolute-crash-chrome-004.html
./css/css-position/position-absolute-crash-chrome-008.html
./css/css-inline/inline-crash-chrome-001.html
./css/css-cascade/scope-implicit-crash-print.html
./css/css-sizing/min-max-content-orthogonal-flow-crash-001.html
./css/css-images/infinite-radial-gradient-refcrash.html
./css/css-images/infinite-radial-gradient-crash-ref.html
./css/css-images/gradient-refcrash.html
./css/css-fonts/crash-large-grapheme-cluster.html
./css/css-fonts/crash-font-face-invalid-descriptor.html
./css/css-pseudo/first-letter-of-html-root-refcrash.html
./css/css-pseudo/first-letter-of-html-root-crash-ref.html
./css/css-pseudo/before-in-display-none-thcrash.html
./css/css-typed-om/set-var-reference-thcrash.html
./css/css-flexbox/inline-flexbox-vertical-rl-image-flexitem-crash-print.html
./css/css-scoping/whitespace-crash-001.html
./css/CSS2/linebox/inline-negative-margin-minmax-crash-001.html
./css/CSS2/linebox/inline-children-root-linebox-crash-001.html
./css/css-tables/tfoot-crash-print.html
./css/css-layout-api/crash-multicol.https.html
./css/css-text/white-space/append-whitespace-only-node-crash-001.html
./css/css-text/overflow-wrap/overflow-wrap-break-word-white-space-crash-002.html
./css/css-text/word-break/word-break-break-word-crash-001.html
./css/css-ui/text-overflow-ellipsis-abspos-in-inline-block-crash-001.html
./css/css-grid/chrome-crash-001.html
./css/css-contain/contain-chrome-thcrash-001.html
./css/printing/table-overflow-quirks-frameset-crash-print.html
./css/printing/destination-backslash-crash-print.html
./css/printing/page-overflow-crash-print.html
./css/printing/huge-font-crash-print.html
./css/css-box/box-chrome-crash-001.html
./css/css-view-transitions/no-crash-view-transition-in-massive-iframe.html
./css/css-view-transitions/view-transition-types-mutable-no-document-element-crashtest.html
./css/css-view-transitions/no-crash-set-exception.html
./streams/readable-streams/cross-realm-crash.window.js
./html/canvas/offscreen/pixel-manipulation/2d.imageData.get.large.crash.worker.js
./html/canvas/offscreen/pixel-manipulation/2d.imageData.get.large.crash.html
./html/canvas/element/pixel-manipulation/2d.imageData.get.large.crash.html
./html/browsers/browsing-the-web/navigating-across-documents/resources/unknown-protocol-reload-crash-frame.html
./html/browsers/origin/origin-keyed-agent-clusters/resources/crashy-popup.sub.html
./html/browsers/origin/origin-keyed-agent-clusters/resources/crashy-popup.sub.html.headers
./html/semantics/embedded-content/the-video-element/video_crash_empty_src.html
./html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-alt-crash-001.html
./interfaces/crash-reporting.idl
./dom/nodes/remove-and-adopt-thcrash.html
./svg/print/svg-use-page-break-crash-print.html
./svg/pservers/pattern-with-invalid-base-cloned-thcrash.html
./selection/selection-shadow-dom-crash-print.html
./payment-request/payment-request-constructor-thcrash.https.html

[WeizhongX]

@jgraham @gsnedders

[gsnedders]

I feel like this has been talked about previously somewhere, but I can't find where. Maybe around the time that 50ae113 landed? Ah, the answer is largely on #20017, the PR that commit came from.

On the face of it, the worst case here is something that's accidentally just a support file and therefore not a test at all (because any other test type — whether a testharness test or a reftest — requires an author to write specific markup for it to be considered that test type).

I think there's two arguments for a lint rule that is broader than that which disallows "crash" in any non-crashtest test filename:

1. Confusion about how tests are detected (which seems to be the case with css/css-inline/inline-crash-chrome-001.html in https://issues.chromium.org/issues/379764806), because the -crash here is a total red herring.
2. The fact that "crash" is just not a very descriptive name for a test which has an actual pass condition.

Related to this, looking at https://github.com/web-platform-tests/wpt/blob/475127f90be9926867796ff98717b621a358af52/css/css-inline/inline-crash-chrome-001.html makes me wonder if we should lint for testharness tests which have no asserts (or fetch_tests_from_window, etc.) — but we probably can't, because there's enough tests where the actual asserts live in a support file, and thus we'd have loads of false-negatives.

[gsnedders]

Related to this, looking at https://github.com/web-platform-tests/wpt/blob/475127f90be9926867796ff98717b621a358af52/css/css-inline/inline-crash-chrome-001.html makes me wonder if we should lint for testharness tests which have no asserts (or fetch_tests_from_window, etc.) — but we probably can't, because there's enough tests where the actual asserts live in a support file, and thus we'd have loads of false-negatives.

% find . -iname '*crash*' -and -not -name '*-crash.html' -print0 | xargs -0 rg --files-with-matches testharness.js --null | xargs -0 rg --files-without-match assert | sort
./css/CSS2/linebox/inline-children-root-linebox-crash-001.html
./css/CSS2/linebox/inline-negative-margin-minmax-crash-001.html
./css/css-fonts/crash-font-face-invalid-descriptor.html
./css/css-fonts/crash-large-grapheme-cluster.html
./css/css-multicol/with-custom-layout-on-same-element-crash.https.html
./css/css-position/position-absolute-crash-chrome-001.html
./css/css-position/position-absolute-crash-chrome-010.html
./css/css-position/position-absolute-crash-chrome-011.html
./css/css-position/position-absolute-crash-chrome-012.html
./css/css-position/position-absolute-crash-chrome-013.html
./css/css-sizing/min-max-content-orthogonal-flow-crash-001.html
./css/css-text/overflow-wrap/overflow-wrap-break-word-white-space-crash-002.html
./css/css-text/white-space/append-whitespace-only-node-crash-001.html
./css/css-text/word-break/word-break-break-word-crash-001.html
./html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfer.to.imagebitmap.nocrash.html
./html/semantics/embedded-content/the-video-element/video_crash_empty_src.html
./svg/pservers/pattern-with-invalid-base-cloned-thcrash.html

Most of these are admittedly older than WPT having actual crash test support.