In this part, I want to cover how I would tackle an issue labeled E-needs-test.
I'll also be mentoring a Google Summer of Code project this year, and it's test-suite related - so if you're interested, you can read more about it here
Given that this isn't a very time-consuming task, I expect this post won't be very long. I'll document everything as I go - you'll see my actual thought process and decisions in real-time
Selecting and Studying an Issue
Let's head over to GitHub to search for issues. Looking at the list, I'll pick the first available issue. I don't think it will make a big difference, and to be honest, picking one at random for content isn't a bad idea either
So I've selected an issue, now let's see what's going on. According to the issue description, this code used to cause the compiler to panic when trying to compile it. However, according to the last comment, the problem was fixed in one of the pull requests, even though the issue itself is still open. Therefore, we want to add a test using this code example that used to panic, to lock in the new correct behavior and ensure that it now works as expected
Tackling the Problem
So, now that we've looked into the issue's context, let's start taking action. Let's open our code editor, navigate to our forked repository, and create a new branch for our pull request. I'm just going to do what I normally do (to be honest, I still don't feel very confident using Git, so I might do something a bit odd or not the usual way):
If you're also not confident with Git, that's totally fine - the commands I show here work reliably, and the worst case is that reviewers will help you fix any Git issues
$ git checkout -b add-regression-test-1
And also, to ensure I've a clean and up-to-date branch, I run:
$ git fetch upstream
$ git reset --hard upstream/main
Fair warning:
reset --hardwill discard all your uncommitted local changes. Make sure you don't have any unsaved work before running this command. I always verify this first, which is why I use it safely
This fetches the latest changes from the main Rust repository and resets our branch to match it exactly
Now let's figure out where to add our test. Since it'll be a UI test, I should find the appropriate subdirectory to place it. Since the issue was titled "resolve: no entry found for key", it's clear this is a name resolution problem, which means the test should go in the resolve directory
Also a very small tip on how can you determine the directory, because in your case it might not be specified in the issue's title, in that case feel free to look at labels on that issue, it's very likely do have labels like A-resolve or A-macros like in my case, and you can search for this words in tests/ui/README.md, also, try to use your own intuition to understand what is tested in that test (sometimes it's not very obvious, I completely relate that part)
Let's open tests/ui/README.md and search for "resolve", to see what's available:
$ code tests/ui/README.md
I use Ctrl+F to search for 'resolve'. Aha, there it is: there's a directory with the exact name I need:
tests/ui/resolve/: Name resolution
That's exactly what I need! Now let's think of a name for the test. It should be short and reflect what's being tested. To help with this, let's look at the code for the future test, which is taken from the example:
#![feature(decl_macro)]
macro_rules! exported {
() => {
#[macro_export]
macro_rules! exported {
() => {};
}
};
}
use inner1::*;
exported!();
mod inner1 {
pub macro exported() {}
}
This is some really convoluted nested macro code! Honestly, I have no clue what could have gone wrong here. I tried reading yaahc's comments on the issue, but they didn't give me much insight, and I must admit I'm hearing about the decl_macro feature almost for the first time
I honestly don't fully understand what this code does, but for adding a regression test, I don't need to - the bug was already fixed, I just need to make sure this code compiles now
I'll make a relatively generic test name and adding the issue number at the end. Something like exported-macro-in-mod-147958 - honestly, I just wrote down what I see in the test itself
$ touch tests/ui/resolve/exported-macro-in-mod-147958.rs
$ code tests/ui/resolve/exported-macro-in-mod-147958.rs
So I just paste the code above into this file. The only difference is that I'll add a directive at the very top of the test:
//@ check-pass
In UI tests, if you don't specify a directive like //@ check-pass, the harness assumes the test should produce errors and compares against a .stderr file
I won't go into detail about test directives here, because there's already good material on this topic. Our focus is a bit different, but I highly recommend checking out the Rustc dev guide on directives
Okay, now that the test is ready, let's try running it:
$ ./x test tests/ui/resolve/exported-macro-in-mod-147958.rs
This will start building the compiler and any necessary tools. By the way, since I expect no errors to occur, a .stderr file shouldn't be generated, so I didn't use the --bless flag (the one that generates the error output file)
Our test didn't pass...
running 1 tests
[ui] tests/ui/resolve/exported-macro-in-mod-147958.rs ... F
failures:
---- [ui] tests/ui/resolve/exported-macro-in-mod-147958.rs stdout ----
Saved the actual stderr to `/home/gh-Kivooeo/r/rust/build/aarch64-unknown-linux-gnu/test/ui/resolve/exported-macro-in-mod-147958/exported-macro-in-mod-147958.stderr`
normalized stderr:
error[E0601]: `main` function not found in crate `exported_macro_in_mod_147958`
--> $DIR/exported-macro-in-mod-147958.rs:15:2
|
LL | }
| ^ consider adding a `main` function to `$DIR/exported-macro-in-mod-147958.rs`
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0601`.
Ah, I forgot to add a main function. Of course, I could have added a directive like //@ compile-flags: --crate-type lib to indicate this is a library, but I'm more used to just adding an empty main function
Alright, that's it. I can also run ./x test tidy just to make sure there's no trailing whitespace and that I didn't forget a newline at the end of the file
tidy checks formatting, whitespace, file placement, and other repository conventions
Now I can prepare the pull request. To do that I ran the following commands:
user@machine ~/r/rust (add-regression-test-1)> git add .
user@machine ~/r/rust (add-regression-test-1)> git commit -m "add regression test"
user@machine ~/r/rust (add-regression-test-1)> git push --force-with-lease origin add-regression-test-1
Note:
--force-with-leaseisn't needed for a new branch, but I use it out of habit
I think everything is ready, so let's go to GitHub. I'll open my fork, switch to the branch I created for this pull request, and click the big "Contribute -> Open pull request" button
I'll keep the description minimal since everything is covered here. I titled it "add regression test for 147958" and I just left the description as "fixes #147958", so that the issue will be closed after merging the pull request
Since I don't know who should do the review, I left it like that. Usually, you can write something like r? team (e.g. r? compiler) or a specific member like r? Kivooeo. If you don't specify anyone, rustbot will pick the most appropriate free reviewer in rotation
And that's it. Our pull request was created here. Rustbot assigned TaKO8Ki as the reviewer, so now I wait for review. I'm not 100% sure about this test since I'm not familiar with this feature, but I fully expect it to be approved. You can also check the reviewer's GitHub profile to see their recent activity - I often do this myself, although I don't really have any specific expectations. Reviews can take up to two weeks before you can re-request a review if needed
Given the size and simplicity of this PR, I'm expecting a pretty quick approval. The most likely outcome is that another contributor will approve it or suggest corrections
An Oversight
About 30 minutes after I created the pull request, I remembered that I forgot to add a most important comment
//! Regression test for <https://github.com/rust-lang/rust/issues/147958>
On top of the test, so in future it would be easier to navigate for original issue, let's quickly fix it!
So all I do is just add this comment, final test looks this
//! Regression test for <https://github.com/rust-lang/rust/issues/147958>
//@ check-pass
#![feature(decl_macro)]
macro_rules! exported {
() => {
#[macro_export]
macro_rules! exported {
() => {};
}
};
}
use inner1::*;
exported!();
mod inner1 {
pub macro exported() {}
}
fn main() {}
Here I have an empty main function, and a comment with link to the issue, btw, worth noting that while many regression tests don't include this comment (and that's totally okay), I personally find it really helpful to have the direct link. I also pretty often see comment like:
//! Test for #31233
Which makes it pretty hard to follow an actual issue, ctrl + click by link is way faster and easier
Next what I do is some git magic, at least this is how I see this
$ git add .
$ git commit --amend --no-edit
$ git push --force-with-lease origin add-regression-test-1
Now that's 100% ready :)
What to expect during review
Now let's talk a bit about what you can expect from the review process itself. I won't be able to show this part directly in the blog post, but readers can check out the pull request I created above to see if there's any review activity. I fully expect there might be some requested changes to my pull request, but let me describe the most common scenarios that might happen after you create your PR:
-
CI fails due to tidy issues:
- There might be trailing whitespaces
- You forgot to add a newline at the end of the file
- You created a test in the root of
tests/ui- which has been recently prohibited, tests must now be in subdirectories - You used an unclear test name - we now discourage names like
issue-123.rs
-
You'll be asked to rename the test, and the reviewer will likely suggest a more appropriate name
-
You'll be asked to move the test to a more suitable directory
-
You'll be asked to modify the test in some way:
- Add a comment
- Change or add directives
- Adjust the test code itself
These are all completely normal situations that just need small fixes. Think of review feedback as collaborative teamwork and suggestions for improvement, not as criticism of your work. Every contributor goes through this!
Conclusion
That's it! We went from finding an issue to submitting a PR in about 15-20 minutes. E-needs-test issues are perfect first contributions because:
- They're low-risk (just adding a test, not changing compiler logic)
- The solution is usually straightforward (the bug is already fixed)
- You get familiar with the test suite structure
- You learn the PR workflow without much pressure
It is important to add at the end that, depending on the issue itself, the process may differ slightly, for example, a different directory for the test or the test will expect an error output (i.e., the creation of a .stderr file), so you may have questions. If you want to try adding a test or solving an issue labeled E-needs-test, but are stuck or feel like you don't know something, you can write to me in private messages on Zulip and/or assign me as a reviewer using r? Kivooeo in the description. Thank you to everyone who works on Rust and everyone who will work on it in the future!
In the next part I'm planning to take some A-diagnostics issue, to actually fix some error messages, yay!
On a side note, if you want to support the release of future blog posts, my work on the compiler, mentoring newcomers and GSoC students, I'd be very grateful if you could take a look at my sponsor page <3