Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement @snapshot check for htmldocck #91209

Merged
merged 2 commits into from
Dec 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 76 additions & 4 deletions src/etc/htmldocck.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,20 @@
highlights for example. If you want to simply check for the presence of
a given node or attribute, use an empty string (`""`) as a `PATTERN`.

* `@count PATH XPATH COUNT' checks for the occurrence of the given XPath
* `@count PATH XPATH COUNT` checks for the occurrence of the given XPath
in the specified file. The number of occurrences must match the given
count.

* `@snapshot NAME PATH XPATH` creates a snapshot test named NAME.
A snapshot test captures a subtree of the DOM, at the location
determined by the XPath, and compares it to a pre-recorded value
in a file. The file's name is the test's name with the `.rs` extension
replaced with `.NAME.html`, where NAME is the snapshot's name.

htmldocck supports the `--bless` option to accept the current subtree
as expected, saving it to the file determined by the snapshot's name.
compiletest's `--bless` flag is forwarded to htmldocck.

* `@has-dir PATH` checks for the existence of the given directory.

All conditions can be negated with `!`. `@!has foo/type.NoSuch.html`
Expand Down Expand Up @@ -137,6 +147,10 @@

channel = os.environ["DOC_RUST_LANG_ORG_CHANNEL"]

# Initialized in main
rust_test_path = None
bless = None

class CustomHTMLParser(HTMLParser):
"""simplified HTML parser.

Expand Down Expand Up @@ -387,6 +401,32 @@ def get_tree_count(tree, path):
return len(tree.findall(path))


def check_snapshot(snapshot_name, tree):
assert rust_test_path.endswith('.rs')
snapshot_path = '{}.{}.{}'.format(rust_test_path[:-3], snapshot_name, 'html')
try:
with open(snapshot_path, 'r') as snapshot_file:
expected_str = snapshot_file.read()
except FileNotFoundError:
if bless:
expected_str = None
else:
raise FailedCheck('No saved snapshot value')

actual_str = ET.tostring(tree).decode('utf-8')

if expected_str != actual_str:
if bless:
with open(snapshot_path, 'w') as snapshot_file:
snapshot_file.write(actual_str)
else:
print('--- expected ---\n')
print(expected_str)
print('\n\n--- actual ---\n')
print(actual_str)
print()
raise FailedCheck('Actual snapshot value is different than expected')

def stderr(*args):
if sys.version_info.major < 3:
file = codecs.getwriter('utf-8')(sys.stderr)
Expand Down Expand Up @@ -448,6 +488,28 @@ def check_command(c, cache):
ret = expected == found
else:
raise InvalidCheck('Invalid number of @{} arguments'.format(c.cmd))

elif c.cmd == 'snapshot': # snapshot test
if len(c.args) == 3: # @snapshot <snapshot-name> <html-path> <xpath>
[snapshot_name, html_path, pattern] = c.args
camelid marked this conversation as resolved.
Show resolved Hide resolved
tree = cache.get_tree(html_path)
xpath = normalize_xpath(pattern)
subtrees = tree.findall(xpath)
if len(subtrees) == 1:
[subtree] = subtrees
try:
check_snapshot(snapshot_name, subtree)
ret = True
except FailedCheck as err:
cerr = str(err)
ret = False
elif len(subtrees) == 0:
raise FailedCheck('XPATH did not match')
else:
raise FailedCheck('Expected 1 match, but found {}'.format(len(subtrees)))
else:
raise InvalidCheck('Invalid number of @{} arguments'.format(c.cmd))

elif c.cmd == 'has-dir': # has-dir test
if len(c.args) == 1: # @has-dir <path> = has-dir test
try:
Expand All @@ -458,11 +520,13 @@ def check_command(c, cache):
ret = False
else:
raise InvalidCheck('Invalid number of @{} arguments'.format(c.cmd))

elif c.cmd == 'valid-html':
raise InvalidCheck('Unimplemented @valid-html')

elif c.cmd == 'valid-links':
raise InvalidCheck('Unimplemented @valid-links')

else:
raise InvalidCheck('Unrecognized @{}'.format(c.cmd))

Expand All @@ -483,11 +547,19 @@ def check(target, commands):


if __name__ == '__main__':
if len(sys.argv) != 3:
stderr('Usage: {} <doc dir> <template>'.format(sys.argv[0]))
if len(sys.argv) not in [3, 4]:
stderr('Usage: {} <doc dir> <template> [--bless]'.format(sys.argv[0]))
raise SystemExit(1)

check(sys.argv[1], get_commands(sys.argv[2]))
rust_test_path = sys.argv[2]
if len(sys.argv) > 3 and sys.argv[3] == '--bless':
bless = True
else:
# We only support `--bless` at the end of the arguments.
# This assert is to prevent silent failures.
assert '--bless' not in sys.argv
bless = False
check(sys.argv[1], get_commands(rust_test_path))
if ERR_COUNT:
stderr("\nEncountered {} errors".format(ERR_COUNT))
raise SystemExit(1)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="docblock"><p>Hello world!
Goodbye!
Hello again!</p>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="docblock"><p>Hello world!</p>
<p>Goodbye!
Hello again!</p>
</div>
12 changes: 2 additions & 10 deletions src/test/rustdoc/mixing-doc-comments-and-attrs.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
#![crate_name = "foo"]

// @has 'foo/struct.S1.html'
// @count - '//details[@class="rustdoc-toggle top-doc"]/div[@class="docblock"]/p' \
// 1
// @has - '//details[@class="rustdoc-toggle top-doc"]/div[@class="docblock"]/p[1]' \
// 'Hello world! Goodbye! Hello again!'
// @snapshot S1_top-doc - '//details[@class="rustdoc-toggle top-doc"]/div[@class="docblock"]'

#[doc = "Hello world!\n\n"]
/// Goodbye!
#[doc = " Hello again!\n"]
pub struct S1;

// @has 'foo/struct.S2.html'
// @count - '//details[@class="rustdoc-toggle top-doc"]/div[@class="docblock"]/p' \
// 2
// @has - '//details[@class="rustdoc-toggle top-doc"]/div[@class="docblock"]/p[1]' \
// 'Hello world!'
// @has - '//details[@class="rustdoc-toggle top-doc"]/div[@class="docblock"]/p[2]' \
// 'Goodbye! Hello again!'
// @snapshot S2_top-doc - '//details[@class="rustdoc-toggle top-doc"]/div[@class="docblock"]'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make more sense to keep the original order for the argument? So first, the file we're checking and then the snapshot file. It would be incoherent with the rest otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, but then it looks a bit like S2_top-doc is a text pattern to match against. Plus, the name of the snapshot is important so you know what file to look at it for the output, so I put it at the front. I'm open to changing it, but I'm not sure if putting the snapshot name at the end would necessarily be clearer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tihnk coherence is the most important thing. For me it's super strange that only @snapshot has this order for the args. If others agree with you, I won't oppose though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Joshua gave a thumbs up on my previous message, so I think let's keep it at the beginning for now if that's okay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can always be changed later on in any case so it's fine.


/// Hello world!
///
Expand Down
12 changes: 6 additions & 6 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2217,12 +2217,12 @@ impl<'test> TestCx<'test> {
self.check_rustdoc_test_option(proc_res);
} else {
let root = self.config.find_rust_src_root().unwrap();
let res = self.cmd2procres(
Command::new(&self.config.docck_python)
.arg(root.join("src/etc/htmldocck.py"))
.arg(&out_dir)
.arg(&self.testpaths.file),
);
let mut cmd = Command::new(&self.config.docck_python);
cmd.arg(root.join("src/etc/htmldocck.py")).arg(&out_dir).arg(&self.testpaths.file);
if self.config.bless {
cmd.arg("--bless");
}
let res = self.cmd2procres(&mut cmd);
if !res.status.success() {
self.fatal_proc_rec_with_ctx("htmldocck failed!", &res, |mut this| {
this.compare_to_default_rustdoc(&out_dir)
Expand Down