Regression: rustdoc --test stopped running doctests in sections with backtick syntax in the title

[SimonSapin]

Test case a.md:

# `foo`

```rust
assert!(true)
```

Output with rustdoc 1.24.0-beta.12 (ed2c0f0 2018-02-12) and rustdoc 1.25.0-nightly (3ec5a99 2018-02-14)

% rustdoc +beta --test /tmp/a.md

running 1 test
test /tmp/a.md - _code_foo__code_ (line 3) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

% rustdoc +nightly --test /tmp/a.md
WARNING: /tmp/a.md - foo (line 3) Code block is not currently run as a test, but will in future versions of rustdoc. Please ensure this code block is a runnable test, or use the `ignore` directive.

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

[GuillaumeGomez]

I think this issue should be solved by the removal of hoedown but it needs to be confirmed.

[SimonSapin]

Confirmed fixed in rustdoc 1.25.0-nightly (27a046e 2018-02-18), thanks.

[ollie27]

Won't this affect the 1.25 stable release?

[SimonSapin]

@alexcrichton Has 1.25 beta already been branched? If so, should the fix for this be back-ported?

@GuillaumeGomez Is #48274 what fixed this?

[QuietMisdreavus]

@ollie27 Yes. The beta that was tested in the OP is the one that was just promoted to stable. The 1.25 beta itself is currently blocked (per discussion in #rust-release) so these warnings will still affect the 1.25 cycle. Backporting the entire removal of Hoedown is probably ill-advised, though.

A quick investigation tells me that these warnings were "introduced" by the PR that switched Pulldown on by default... but only because they were already being shown in Pulldown mode to begin with. Furthermore, the warnings only show when testing a standalone Markdown file, not when the exact same markdown is part of a Rust crate.

Farther discussion in #rustdoc pins this to the way headers are parsed to create doctest names. Since doctests in crates are named after the item they're documenting, this code only runs when testing standalone Markdown files. Also, after rereading the issue description, i totally missed that this wasn't about spurious warnings, but instead about doctests not being run in the first place.

I've done some digging into this issue, but @GuillaumeGomez believes this feature is not popular enough to fix, as the fix will land in the next version anyway. Backporting the PR that fixed it (totally removing Hoedown) is not feasible, since we're relying on the release timing in the first place. Therefore, if we were to apply a fix to beta, it would have to be specific to beta, and would need to be resolved at the next beta merger. However, i would rather this bug not be exposed for a full release cycle, even if it would never actually be hit "in the wild".

[QuietMisdreavus]

From what i can tell, the issue comes out because it does find the test, but the way the test is named internally is different between the renderers. Therefore, when it tries to register the test, it doesn't find a match, and decides to bail instead.

rust/src/librustdoc/test.rs

Lines 522 to 536 in da1f1b5

	if self.render_type == RenderType::Pulldown {
	let name_beg = self.generate_name_beginning(&filename);
	let mut found = false;
	let test = test.trim().to_owned();
	if let Some(entry) = self.old_tests.get_mut(&name_beg) {
	found = entry.remove_item(&test).is_some();
	}
	if !found {
	eprintln!("WARNING: {} Code block is not currently run as a test, but will \
	in future versions of rustdoc. Please ensure this code block is \
	a runnable test, or use the `ignore` directive.",
	name);
	return
	}
	}

It seems that Hoedown receives a fully-rendered inner-text for the header when it gets the header call...

rust/src/librustdoc/html/markdown.rs

Lines 957 to 972 in da1f1b5

	extern fn header(_ob: *mut hoedown_buffer,
	text: *const hoedown_buffer,
	level: libc::c_int, data: *const hoedown_renderer_data,
	_: libc::size_t) {
	unsafe {
	let opaque = (*data).opaque as *mut hoedown_html_renderer_state;
	let tests = &mut *((*opaque).opaque as *mut ::test::Collector);
	if text.is_null() {
	tests.register_header("", level as u32);
	} else {
	let text = (*text).as_bytes();
	let text = str::from_utf8(text).unwrap();
	tests.register_header(text, level as u32);
	}
	}
	}

...but the Pulldown version of this code only selects for the first Text event it receives, bypassing any other events (like a wrapping <code> span) that would have happened in between:

rust/src/librustdoc/html/markdown.rs

Lines 1043 to 1054 in da1f1b5

	Event::Start(Tag::Header(level)) => {
	register_header = Some(level as u32);
	}
	Event::Text(ref s) if register_header.is_some() => {
	let level = register_header.unwrap();
	if s.is_empty() {
	tests.register_header("", level);
	} else {
	tests.register_header(s, level);
	}
	register_header = None;
	}

This seems to reach as far back as the introduction of Pulldown.

A proper fix for this would be to resolve the difference in these received names, so that when the latter two samples call register_header they call it with the same text for each heading.

[QuietMisdreavus]

Confirming the issue occurs with the freshly-released beta:

$ rustdoc +beta --test a.md
WARNING: a.md - foo (line 3) Code block is not currently run as a test, but will in future versions of rustdoc. Please ensure this code block is a runnable test, or use the `ignore` directive.

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ rustdoc +beta --version
rustdoc 1.25.0-beta.2 (1e8fbb143 2018-02-19)

[Mark-Simulacrum]

Fix is merged, closing.

[SimonSapin]

Verified fixed in 1.25.0-beta.7, thanks.