rust-lang / rust-lang/rustfmt

`idempotence_tests` are unstable when setting inline rustfmt comments

Open
#6,011 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

A-config A-test-suite C-bug I-non-idempotency
Dominant language
Rust
Stars
7k
Forks
1.1k
Avg merge
2d 13h
Merged PRs (30d)
24

Description

As I was generating the code for #6000 I noticed that when setting config options with rustfmt comments the test would become unstable. Sometimes the test would pass and sometimes the same test would fail without changing any code. We believe this is down to the order that the config items get applied. Setting one config param before the other can cause a conflict and incorrect setup for the test.

To resolve this, we moved the test's config into a .toml file (as can be seen in the MR) which applies them in the correct order.

More deatils can be found in #6000 and the comment here. I've pasted some of it below for quick reference.

My hunch is that the issue is related to setting the configs inline with the rustfmt comments like //rustfmt-{key}: {value} in read_config. From what I can tell read_significant_comments returns a HashMap<String, String>, and when we try to iterate over it we don't necessarily iterate over the configurations in the same order that they're listed in the file. Sometimes we set the max_width=120 before we set attr_fn_like_width=120, and in that case everything works out. In other cases we try to set attr_fn_like_width=120 first, and that fails since we don't allow any width configuration to exceed the max_width, which is still set to its default of 100. so attr_fn_like_width would also get set to 100.

How to reproduce?

Run the following test script. Run it multiple times...sometimes it passes and someetimes it fails.

https://github.com/rust-lang/rustfmt/blob/fb36c1438c5f451fb4c4b2ae90c4fab41bf9f345/tests/target/issue-5801/attribute_does_not_wrap_within_max_width.rs#L1-L10

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with read_config in src/test/mod.rs around lines 691-712 and the reproduction at tests/target/issue-5801/attribute_does_not_wrap_within_max_width.rs. Check how significant comments are applied from the HashMap, then run the test repeatedly. Done means the configuration is applied deterministically and the test no longer alternates between passing and failing.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
testing, tooling
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.