397 lines
20 KiB
HTML
397 lines
20 KiB
HTML
<!DOCTYPE HTML>
|
|
<html lang="en" class="light sidebar-visible" dir="ltr">
|
|
<head>
|
|
<!-- Book generated using mdBook -->
|
|
<meta charset="UTF-8">
|
|
<title>Best practices - Rust Compiler Development Guide</title>
|
|
|
|
|
|
<!-- Custom HTML head -->
|
|
|
|
<meta name="description" content="A guide to developing the Rust compiler (rustc)">
|
|
<meta name="viewport" content="width=device-width, initial-scale=1">
|
|
<meta name="theme-color" content="#ffffff">
|
|
|
|
<link rel="icon" href="../favicon.svg">
|
|
<link rel="shortcut icon" href="../favicon.png">
|
|
<link rel="stylesheet" href="../css/variables.css">
|
|
<link rel="stylesheet" href="../css/general.css">
|
|
<link rel="stylesheet" href="../css/chrome.css">
|
|
<link rel="stylesheet" href="../css/print.css" media="print">
|
|
|
|
<!-- Fonts -->
|
|
<link rel="stylesheet" href="../FontAwesome/css/font-awesome.css">
|
|
<link rel="stylesheet" href="../fonts/fonts.css">
|
|
|
|
<!-- Highlight.js Stylesheets -->
|
|
<link rel="stylesheet" id="highlight-css" href="../highlight.css">
|
|
<link rel="stylesheet" id="tomorrow-night-css" href="../tomorrow-night.css">
|
|
<link rel="stylesheet" id="ayu-highlight-css" href="../ayu-highlight.css">
|
|
|
|
<!-- Custom theme stylesheets -->
|
|
|
|
|
|
<!-- Provide site root and default themes to javascript -->
|
|
<script>
|
|
const path_to_root = "../";
|
|
const default_light_theme = "light";
|
|
const default_dark_theme = "navy";
|
|
</script>
|
|
<!-- Start loading toc.js asap -->
|
|
<script src="../toc.js"></script>
|
|
</head>
|
|
<body>
|
|
<div id="body-container">
|
|
<!-- Work around some values being stored in localStorage wrapped in quotes -->
|
|
<script>
|
|
try {
|
|
let theme = localStorage.getItem('mdbook-theme');
|
|
let sidebar = localStorage.getItem('mdbook-sidebar');
|
|
|
|
if (theme.startsWith('"') && theme.endsWith('"')) {
|
|
localStorage.setItem('mdbook-theme', theme.slice(1, theme.length - 1));
|
|
}
|
|
|
|
if (sidebar.startsWith('"') && sidebar.endsWith('"')) {
|
|
localStorage.setItem('mdbook-sidebar', sidebar.slice(1, sidebar.length - 1));
|
|
}
|
|
} catch (e) { }
|
|
</script>
|
|
|
|
<!-- Set the theme before any content is loaded, prevents flash -->
|
|
<script>
|
|
const default_theme = window.matchMedia("(prefers-color-scheme: dark)").matches ? default_dark_theme : default_light_theme;
|
|
let theme;
|
|
try { theme = localStorage.getItem('mdbook-theme'); } catch(e) { }
|
|
if (theme === null || theme === undefined) { theme = default_theme; }
|
|
const html = document.documentElement;
|
|
html.classList.remove('light')
|
|
html.classList.add(theme);
|
|
html.classList.add("js");
|
|
</script>
|
|
|
|
<input type="checkbox" id="sidebar-toggle-anchor" class="hidden">
|
|
|
|
<!-- Hide / unhide sidebar before it is displayed -->
|
|
<script>
|
|
let sidebar = null;
|
|
const sidebar_toggle = document.getElementById("sidebar-toggle-anchor");
|
|
if (document.body.clientWidth >= 1080) {
|
|
try { sidebar = localStorage.getItem('mdbook-sidebar'); } catch(e) { }
|
|
sidebar = sidebar || 'visible';
|
|
} else {
|
|
sidebar = 'hidden';
|
|
}
|
|
sidebar_toggle.checked = sidebar === 'visible';
|
|
html.classList.remove('sidebar-visible');
|
|
html.classList.add("sidebar-" + sidebar);
|
|
</script>
|
|
|
|
<nav id="sidebar" class="sidebar" aria-label="Table of contents">
|
|
<!-- populated by js -->
|
|
<mdbook-sidebar-scrollbox class="sidebar-scrollbox"></mdbook-sidebar-scrollbox>
|
|
<noscript>
|
|
<iframe class="sidebar-iframe-outer" src="../toc.html"></iframe>
|
|
</noscript>
|
|
<div id="sidebar-resize-handle" class="sidebar-resize-handle">
|
|
<div class="sidebar-resize-indicator"></div>
|
|
</div>
|
|
</nav>
|
|
|
|
<div id="page-wrapper" class="page-wrapper">
|
|
|
|
<div class="page">
|
|
<div id="menu-bar-hover-placeholder"></div>
|
|
<div id="menu-bar" class="menu-bar sticky">
|
|
<div class="left-buttons">
|
|
<label id="sidebar-toggle" class="icon-button" for="sidebar-toggle-anchor" title="Toggle Table of Contents" aria-label="Toggle Table of Contents" aria-controls="sidebar">
|
|
<i class="fa fa-bars"></i>
|
|
</label>
|
|
<button id="theme-toggle" class="icon-button" type="button" title="Change theme" aria-label="Change theme" aria-haspopup="true" aria-expanded="false" aria-controls="theme-list">
|
|
<i class="fa fa-paint-brush"></i>
|
|
</button>
|
|
<ul id="theme-list" class="theme-popup" aria-label="Themes" role="menu">
|
|
<li role="none"><button role="menuitem" class="theme" id="default_theme">Auto</button></li>
|
|
<li role="none"><button role="menuitem" class="theme" id="light">Light</button></li>
|
|
<li role="none"><button role="menuitem" class="theme" id="rust">Rust</button></li>
|
|
<li role="none"><button role="menuitem" class="theme" id="coal">Coal</button></li>
|
|
<li role="none"><button role="menuitem" class="theme" id="navy">Navy</button></li>
|
|
<li role="none"><button role="menuitem" class="theme" id="ayu">Ayu</button></li>
|
|
</ul>
|
|
<button id="search-toggle" class="icon-button" type="button" title="Search. (Shortkey: s)" aria-label="Toggle Searchbar" aria-expanded="false" aria-keyshortcuts="S" aria-controls="searchbar">
|
|
<i class="fa fa-search"></i>
|
|
</button>
|
|
</div>
|
|
|
|
<h1 class="menu-title">Rust Compiler Development Guide</h1>
|
|
|
|
<div class="right-buttons">
|
|
<a href="../print.html" title="Print this book" aria-label="Print this book">
|
|
<i id="print-button" class="fa fa-print"></i>
|
|
</a>
|
|
<a href="https://github.com/rust-lang/rustc-dev-guide" title="Git repository" aria-label="Git repository">
|
|
<i id="git-repository-button" class="fa fa-github"></i>
|
|
</a>
|
|
<a href="https://github.com/rust-lang/rustc-dev-guide/edit/master/src/tests/best-practices.md" title="Suggest an edit" aria-label="Suggest an edit">
|
|
<i id="git-edit-button" class="fa fa-edit"></i>
|
|
</a>
|
|
|
|
</div>
|
|
</div>
|
|
|
|
<div id="search-wrapper" class="hidden">
|
|
<form id="searchbar-outer" class="searchbar-outer">
|
|
<input type="search" id="searchbar" name="searchbar" placeholder="Search this book ..." aria-controls="searchresults-outer" aria-describedby="searchresults-header">
|
|
</form>
|
|
<div id="searchresults-outer" class="searchresults-outer hidden">
|
|
<div id="searchresults-header" class="searchresults-header"></div>
|
|
<ul id="searchresults">
|
|
</ul>
|
|
</div>
|
|
</div>
|
|
|
|
<!-- Apply ARIA attributes after the sidebar and the sidebar toggle button are added to the DOM -->
|
|
<script>
|
|
document.getElementById('sidebar-toggle').setAttribute('aria-expanded', sidebar === 'visible');
|
|
document.getElementById('sidebar').setAttribute('aria-hidden', sidebar !== 'visible');
|
|
Array.from(document.querySelectorAll('#sidebar a')).forEach(function(link) {
|
|
link.setAttribute('tabIndex', sidebar === 'visible' ? 0 : -1);
|
|
});
|
|
</script>
|
|
|
|
<div id="content" class="content">
|
|
<main>
|
|
<h1 id="best-practices-for-writing-tests"><a class="header" href="#best-practices-for-writing-tests">Best practices for writing tests</a></h1>
|
|
<p>This chapter describes best practices related to authoring and modifying tests.
|
|
We want to make sure the tests we author are easy to understand and modify, even
|
|
several years later, without needing to consult the original author and perform
|
|
a bunch of git archeology.</p>
|
|
<p>It's good practice to review the test that you authored by pretending that you
|
|
are a different contributor who is looking at the test that failed several years
|
|
later without much context (this also helps yourself even a few days or months
|
|
later!). Then ask yourself: how can I make my life and their lives easier?</p>
|
|
<p>To help put this into perspective, let's start with an aside on how to write a
|
|
test that makes the life of another contributor as hard as possible.</p>
|
|
<blockquote>
|
|
<p><strong>Aside: Simple Test Sabotage Field Manual</strong></p>
|
|
<p>To make the life of another contributor as hard as possible, one might:</p>
|
|
<ul>
|
|
<li>Name the test after an issue number alone without any other context, e.g.
|
|
<code>issue-123456.rs</code>.</li>
|
|
<li>Have no comments at all on what the test is trying to exercise, no links to
|
|
relevant context.</li>
|
|
<li>Include a test that is massive (that can otherwise be minimized) and
|
|
contains non-essential pieces which distracts from the core thing the test
|
|
is actually trying to test.</li>
|
|
<li>Include a bunch of unrelated syntax errors and other errors which are not
|
|
critical to what the test is trying to check.</li>
|
|
<li>Weirdly format the snippets.</li>
|
|
<li>Include a bunch of unused and unrelated features.</li>
|
|
<li>Have e.g. <code>ignore-windows</code> <a href="./directives.html">compiletest directives</a> but don't offer any
|
|
explanation as to <em>why</em> they are needed.</li>
|
|
</ul>
|
|
</blockquote>
|
|
<h2 id="test-naming"><a class="header" href="#test-naming">Test naming</a></h2>
|
|
<p>Make it easy for the reader to immediately understand what the test is
|
|
exercising, instead of having to type in the issue number and dig through github
|
|
search for what the test is trying to exercise. This has an additional benefit
|
|
of making the test possible to be filtered via <code>--test-args</code> as a collection of
|
|
related tests.</p>
|
|
<ul>
|
|
<li>Name the test after what it's trying to exercise or prevent regressions of.</li>
|
|
<li>Keep it concise.</li>
|
|
<li>Avoid using issue numbers alone as test names.</li>
|
|
<li>Avoid starting the test name with <code>issue-xxxxx</code> prefix as it degrades
|
|
auto-completion.</li>
|
|
</ul>
|
|
<blockquote>
|
|
<p><strong>Avoid using only issue numbers as test names</strong></p>
|
|
<p>Prefer including them as links or <code>#123456</code> in test comments instead. Or if it
|
|
makes sense to include the issue number, also include brief keywords like
|
|
<code>macro-external-span-ice-123956.rs</code>.</p>
|
|
<pre><code class="language-text">tests/ui/typeck/issue-123456.rs // bad
|
|
tests/ui/typeck/issue-123456-asm-macro-external-span-ice.rs // bad (for tab completion)
|
|
tests/ui/typeck/asm-macro-external-span-ice-123456.rs // good
|
|
tests/ui/typeck/asm-macro-external-span-ice.rs // good
|
|
</code></pre>
|
|
<p><code>issue-123456.rs</code> does not tell you immediately anything about what the test
|
|
is actually exercising meaning you need to do additional searching. Including
|
|
the issue number in the test name as a prefix makes tab completion less useful
|
|
(if you <code>ls</code> a test directory and get a bunch of <code>issue-xxxxx</code> prefixes). We
|
|
can link to the issue in a test comment.</p>
|
|
<pre><code class="language-rs">//! Check that `asm!` macro including nested macros that come from external
|
|
//! crates do not lead to a codepoint boundary assertion ICE.
|
|
//!
|
|
//! Regression test for <https://github.com/rust-lang/rust/issues/123456>.
|
|
</code></pre>
|
|
<p>One exception to this rule is <a href="./compiletest.html#crashes-tests">crashes tests</a>: there it is canonical that
|
|
tests are named only after issue numbers because its purpose is to track
|
|
snippets from which issues no longer ICE/crash, and they would either be
|
|
removed or converted into proper ui/other tests in the fix PRs.</p>
|
|
</blockquote>
|
|
<h2 id="test-organization"><a class="header" href="#test-organization">Test organization</a></h2>
|
|
<ul>
|
|
<li>For most test suites, try to find a semantically meaningful subdirectory to
|
|
home the test.
|
|
<ul>
|
|
<li>E.g. for an implementation of RFC 2093 specifically, we can group a
|
|
collection of tests under <code>tests/ui/rfc-2093-infer-outlives/</code>. For the
|
|
directory name, include what the RFC is about.</li>
|
|
</ul>
|
|
</li>
|
|
<li>For the <a href="./compiletest.html#run-make-tests"><code>run-make</code></a> test suite, each <code>rmake.rs</code> must be contained within an
|
|
immediate subdirectory under <code>tests/run-make/</code>. Further nesting is not
|
|
presently supported. Avoid including issue number in the directory name too,
|
|
include that info in a comment inside <code>rmake.rs</code>.</li>
|
|
</ul>
|
|
<h2 id="test-descriptions"><a class="header" href="#test-descriptions">Test descriptions</a></h2>
|
|
<p>To help other contributors understand what the test is about if their changes
|
|
lead to the test failing, we should make sure a test has sufficient docs about
|
|
its intent/purpose, links to relevant context (incl. issue numbers or other
|
|
discussions) and possibly relevant resources (e.g. can be helpful to link to
|
|
Win32 APIs for specific behavior).</p>
|
|
<p><strong>Synopsis of a test with good comments</strong></p>
|
|
<pre><code class="language-rust ignore">//! Brief summary of what the test is exercising.
|
|
//! Example: Regression test for #123456: make sure coverage attribute don't ICE
|
|
//! when applied to non-items.
|
|
//!
|
|
//! Optional: Remarks on related tests/issues, external APIs/tools, crash
|
|
//! mechanism, how it's fixed, FIXMEs, limitations, etc.
|
|
//! Example: This test is like `tests/attrs/linkage.rs`, but this test is
|
|
//! specifically checking `#[coverage]` which exercises a different code
|
|
//! path. The ICE was triggered during attribute validation when we tried
|
|
//! to construct a `def_path_str` but only emitted the diagnostic when the
|
|
//! platform is windows, causing an ICE on unix.
|
|
//!
|
|
//! Links to relevant issues and discussions. Examples below:
|
|
//! Regression test for <https://github.com/rust-lang/rust/issues/123456>.
|
|
//! See also <https://github.com/rust-lang/rust/issues/101345>.
|
|
//! See discussion at <https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/123456-example-topic>.
|
|
//! See [`clone(2)`].
|
|
//!
|
|
//! [`clone(2)`]: https://man7.org/linux/man-pages/man2/clone.2.html
|
|
|
|
//@ ignore-windows
|
|
// Reason: (why is this test ignored for windows? why not specifically
|
|
// windows-gnu or windows-msvc?)
|
|
|
|
// Optional: Summary of test cases: What positive cases are checked?
|
|
// What negative cases are checked? Any specific quirks?
|
|
|
|
fn main() {
|
|
#[coverage]
|
|
//~^ ERROR coverage attribute can only be applied to function items.
|
|
let _ = {
|
|
// Comment highlighting something that deserves reader attention.
|
|
fn foo() {}
|
|
};
|
|
}</code></pre>
|
|
<p>For how much context/explanation is needed, it is up to the author and
|
|
reviewer's discretion. A good rule of thumb is non-trivial things exercised in
|
|
the test deserves some explanation to help other contributors to understand.
|
|
This may include remarks on:</p>
|
|
<ul>
|
|
<li>How an ICE can get triggered if it's quite elaborate.</li>
|
|
<li>Related issues and tests (e.g. this test is like another test but is kept
|
|
separate because...).</li>
|
|
<li>Platform-specific behaviors.</li>
|
|
<li>Behavior of external dependencies and APIs: syscalls, linkers, tools,
|
|
environments and the likes.</li>
|
|
</ul>
|
|
<h2 id="test-content"><a class="header" href="#test-content">Test content</a></h2>
|
|
<ul>
|
|
<li>Try to make sure the test is as minimal as possible.</li>
|
|
<li>Minimize non-critical code and especially minimize unnecessary syntax and type
|
|
errors which can clutter stderr snapshots.</li>
|
|
<li>Where possible, use semantically meaningful names (e.g. <code>fn bare_coverage_attributes() {}</code>).</li>
|
|
</ul>
|
|
<h2 id="flaky-tests"><a class="header" href="#flaky-tests">Flaky tests</a></h2>
|
|
<p>All tests need to strive to be reproducible and reliable. Flaky tests are the
|
|
worst kind of tests, arguably even worse than not having the test in the first
|
|
place.</p>
|
|
<ul>
|
|
<li>Flaky tests can fail in completely unrelated PRs which can confuse other
|
|
contributors and waste their time trying to figure out if test failure is
|
|
related.</li>
|
|
<li>Flaky tests provide no useful information from its test results other than
|
|
it's flaky and not reliable: if a test passed but it's flakey, did I just get
|
|
lucky? if a test is flakey but it failed, was it just spurious?</li>
|
|
<li>Flaky tests degrade confidence in the whole test suite. If a test suite can
|
|
randomly spuriously fail due to flaky tests, did the whole test suite pass or
|
|
did I just get lucky/unlucky?</li>
|
|
<li>Flaky tests can randomly fail in full CI, wasting previous full CI resources.</li>
|
|
</ul>
|
|
<h2 id="compiletest-directives"><a class="header" href="#compiletest-directives">Compiletest directives</a></h2>
|
|
<p>See <a href="./directives.html">compiletest directives</a> for a listing of directives.</p>
|
|
<ul>
|
|
<li>For <code>ignore-*</code>/<code>needs-*</code>/<code>only-*</code> directives, unless extremely obvious,
|
|
provide a brief remark on why the directive is needed. E.g. <code>"//@ ignore-wasi (wasi codegens the main symbol differently)"</code>.</li>
|
|
<li>When using <code>//@ ignore-auxiliary</code>, specify the corresponding main test files,
|
|
e.g. <code>//@ ignore-auxiliary (used by `./foo.rs`)</code>.</li>
|
|
</ul>
|
|
<h2 id="filecheck-best-practices"><a class="header" href="#filecheck-best-practices">FileCheck best practices</a></h2>
|
|
<p>See <a href="https://llvm.org/docs/CommandGuide/FileCheck.html">LLVM FileCheck guide</a> for details.</p>
|
|
<ul>
|
|
<li>Avoid matching on specific register numbers or basic block numbers unless
|
|
they're special or critical for the test. Consider using patterns to match
|
|
them where suitable.</li>
|
|
</ul>
|
|
<blockquote>
|
|
<p><strong>TODO</strong></p>
|
|
<p>Pending concrete advice.</p>
|
|
</blockquote>
|
|
|
|
</main>
|
|
|
|
<nav class="nav-wrapper" aria-label="Page navigation">
|
|
<!-- Mobile navigation buttons -->
|
|
<a rel="prev" href="../tests/adding.html" class="mobile-nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
|
|
<i class="fa fa-angle-left"></i>
|
|
</a>
|
|
|
|
<a rel="next prefetch" href="../tests/compiletest.html" class="mobile-nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
|
|
<i class="fa fa-angle-right"></i>
|
|
</a>
|
|
|
|
<div style="clear: both"></div>
|
|
</nav>
|
|
</div>
|
|
</div>
|
|
|
|
<nav class="nav-wide-wrapper" aria-label="Page navigation">
|
|
<a rel="prev" href="../tests/adding.html" class="nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
|
|
<i class="fa fa-angle-left"></i>
|
|
</a>
|
|
|
|
<a rel="next prefetch" href="../tests/compiletest.html" class="nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
|
|
<i class="fa fa-angle-right"></i>
|
|
</a>
|
|
</nav>
|
|
|
|
</div>
|
|
|
|
|
|
|
|
|
|
<script>
|
|
window.playground_copyable = true;
|
|
</script>
|
|
|
|
|
|
<script src="../elasticlunr.min.js"></script>
|
|
<script src="../mark.min.js"></script>
|
|
<script src="../searcher.js"></script>
|
|
|
|
<script src="../clipboard.min.js"></script>
|
|
<script src="../highlight.js"></script>
|
|
<script src="../book.js"></script>
|
|
|
|
<!-- Custom JS scripts -->
|
|
<script src="../mermaid.min.js"></script>
|
|
<script src="../mermaid-init.js"></script>
|
|
|
|
|
|
</div>
|
|
</body>
|
|
</html>
|