fix parsing bug in treebuilder testsuite, fix crash in scriptdata state (#98)

untitaker · web-flow · commit 1d268b9e1290 · 2024-10-29T22:02:20.000+01:00
diff --git a/src/emitters/callback.rs b/src/emitters/callback.rs
@@ -491,8 +491,14 @@ where
             return false;
         }
 
-        crate::utils::trace_log!("last_start_tag = {:?}", self.emitter_state.last_start_tag);
-        crate::utils::trace_log!("current_tag = {:?}", self.emitter_state.current_tag_name);
+        crate::utils::trace_log!(
+            "current_is_appropriate_end_tag_token: last_start_tag = {:?}",
+            self.emitter_state.last_start_tag
+        );
+        crate::utils::trace_log!(
+            "current_is_appropriate_end_tag_token: current_tag = {:?}",
+            self.emitter_state.current_tag_name
+        );
         self.emitter_state.last_start_tag == self.emitter_state.current_tag_name
     }
 }
diff --git a/src/machine.rs b/src/machine.rs
@@ -418,7 +418,7 @@ pub(crate) mod states {
                 c => {
                     slf.emitter.emit_string(b"</");
                     slf.machine_helper.flush_buffer_characters(&mut slf.emitter);
-                    reconsume_in!(slf, c, Data)
+                    reconsume_in!(slf, c, ScriptData)
                 }
             }
         )
diff --git a/tests/custom-html5lib-tests/tokenizer/custom.test b/tests/custom-html5lib-tests/tokenizer/custom.test
diff --git a/tests/custom-html5lib-tests/tokenizer/error-order-bytes.test b/tests/custom-html5lib-tests/tokenizer/error-order-bytes.test
diff --git a/tests/custom-html5lib-tests/tree-construction/custom.dat b/tests/custom-html5lib-tests/tree-construction/custom.dat
@@ -0,0 +1,10 @@
+#data
+<script></code><a href=foo></a></script>
+#errors
+#document
+| <html>
+|   <head>
+|     <script>
+|       "</code><a href=foo></a>"
+|   <body>
+#errors
diff --git a/tests/html5lib_tokenizer.rs b/tests/html5lib_tokenizer.rs
@@ -385,7 +385,7 @@ fn main() {
         produce_testcases_from_file(&mut tests, &entry.unwrap());
     }
 
-    for entry in glob("tests/custom-html5lib-tests/*.test").unwrap() {
+    for entry in glob("tests/custom-html5lib-tests/tokenizer/*.test").unwrap() {
         produce_testcases_from_file(&mut tests, &entry.unwrap());
     }
 
diff --git a/tests/html5lib_tree_builder.rs b/tests/html5lib_tree_builder.rs
@@ -92,7 +92,11 @@ impl Testcase {
             }
         }
 
-        None
+        if has_errors {
+            Some(rv)
+        } else {
+            None
+        }
     }
 }
 
@@ -138,6 +142,7 @@ fn map_tokenizer_state(input: State) -> html5gum::State {
         State::Plaintext => html5gum::State::PlainText,
         State::RawData(RawKind::Rcdata) => html5gum::State::RcData,
         State::RawData(RawKind::Rawtext) => html5gum::State::RawText,
+        State::RawData(RawKind::ScriptData) => html5gum::State::ScriptData,
         x => todo!("{:?}", x),
     }
 }
@@ -170,7 +175,7 @@ fn build_test(testcase: Testcase, fname: &str, i: usize, scripting: bool) -> Tri
 
             let token_emitter = Html5everEmitter::new(&mut tree_builder);
 
-            let input = &testcase.data[..testcase.data.len() - 1];
+            let input = &testcase.data[..testcase.data.len().saturating_sub(1)];
             let mut tokenizer = Tokenizer::new_with_emitter(input, token_emitter);
             if let Some(state) = initial_state {
                 tokenizer.set_state(state);
@@ -290,6 +295,10 @@ fn main() {
     let args = Arguments::from_args();
     let mut tests = Vec::new();
 
+    for entry in glob("tests/custom-html5lib-tests/tree-construction/*.dat").unwrap() {
+        produce_testcases_from_file(&mut tests, &entry.unwrap());
+    }
+
     for entry in glob("tests/html5lib-tests/tree-construction/*.dat").unwrap() {
         produce_testcases_from_file(&mut tests, &entry.unwrap());
     }
diff --git a/tests/testutils.rs b/tests/testutils.rs
@@ -1,11 +1,51 @@
+use std::backtrace::BacktraceStatus;
 use std::ops::Deref;
-use std::panic::UnwindSafe;
+use std::panic::{self, UnwindSafe};
+use std::sync::Once;
 
 use libtest_mimic::Failed;
 
 use html5gum::testutils::OUTPUT;
 
+/// Improved panic messages for the html5lib-tests test suite.
+///
+/// This function catches panics, prepends the log message to the panic message, and captures
+/// stacktraces.
+///
+/// libtest_mimic already catches panics but doesn't provide stacktraces for some reason.
+///
+/// Because custom test harnesses in Rust do not support capturing of stdout, we have to implement
+/// our own "log buffer", and append it to the error message in case of test failure. OUTPUT is the
+/// log buffer -- it is bound in size and compiled out in release mode. Code can use the
+/// `crate::trace_log` macro to write lines to it.
 pub fn catch_unwind_and_report(f: impl FnOnce() + UnwindSafe) -> Result<(), Failed> {
+    static PANIC_HOOK: Once = Once::new();
+    PANIC_HOOK.call_once(|| {
+        panic::set_hook(Box::new(|_info| {
+            let backtrace = std::backtrace::Backtrace::capture();
+            if backtrace.status() != BacktraceStatus::Captured {
+                html5gum::testutils::trace_log(
+                    "PANIC BACKTRACE: did not capture, use RUST_BACKTRACE=1",
+                );
+            } else {
+                // clean up noisy frames from backtrace
+                let mut backtrace_str = String::new();
+                let mut seen_begin_unwind = false;
+                for line in format!("{:#?}", backtrace).lines() {
+                    if line.contains("\"std::panicking::try::do_call\"") {
+                        break;
+                    } else if seen_begin_unwind {
+                        backtrace_str.push_str(line);
+                        backtrace_str.push('\n');
+                    } else if line.contains("\"core::panicking::panic_fmt\"") {
+                        seen_begin_unwind = true;
+                    }
+                }
+                html5gum::testutils::trace_log(&format!("\nPANIC BACKTRACE:\n{}", backtrace_str));
+            }
+        }));
+    });
+
     let result = std::panic::catch_unwind(f);
 
     let mut msg = String::new();
@@ -28,6 +68,7 @@ pub fn catch_unwind_and_report(f: impl FnOnce() + UnwindSafe) -> Result<(), Fail
                 // If that fails, try to turn it into a &'static str
                 .or_else(|| e.downcast_ref::<&'static str>().map(Deref::deref))
             {
+                msg.push_str("PANIC: ");
                 msg.push_str(s);
             }
 

Original file line number	Diff line number	Diff line change
`@@ -491,8 +491,14 @@ where`
`491`	`491`	`return false;`
`492`	`492`	`}`
`493`	`493`
`494`		`- crate::utils::trace_log!("last_start_tag = {:?}", self.emitter_state.last_start_tag);`
`495`		`- crate::utils::trace_log!("current_tag = {:?}", self.emitter_state.current_tag_name);`
	`494`	`+ crate::utils::trace_log!(`
	`495`	`+ "current_is_appropriate_end_tag_token: last_start_tag = {:?}",`
	`496`	`+ self.emitter_state.last_start_tag`
	`497`	`+ );`
	`498`	`+ crate::utils::trace_log!(`
	`499`	`+ "current_is_appropriate_end_tag_token: current_tag = {:?}",`
	`500`	`+ self.emitter_state.current_tag_name`
	`501`	`+ );`
`496`	`502`	`self.emitter_state.last_start_tag == self.emitter_state.current_tag_name`
`497`	`503`	`}`
`498`	`504`	`}`
Original file line number	Diff line number	Diff line change
`@@ -418,7 +418,7 @@ pub(crate) mod states {`
`418`	`418`	`c => {`
`419`	`419`	`slf.emitter.emit_string(b"</");`
`420`	`420`	`slf.machine_helper.flush_buffer_characters(&mut slf.emitter);`
`421`		`- reconsume_in!(slf, c, Data)`
	`421`	`+ reconsume_in!(slf, c, ScriptData)`
`422`	`422`	`}`
`423`	`423`	`}`
`424`	`424`	`)`
Original file line number	Diff line number	Diff line change
`@@ -385,7 +385,7 @@ fn main() {`
`385`	`385`	`produce_testcases_from_file(&mut tests, &entry.unwrap());`
`386`	`386`	`}`
`387`	`387`
`388`		`- for entry in glob("tests/custom-html5lib-tests/*.test").unwrap() {`
	`388`	`+ for entry in glob("tests/custom-html5lib-tests/tokenizer/*.test").unwrap() {`
`389`	`389`	`produce_testcases_from_file(&mut tests, &entry.unwrap());`
`390`	`390`	`}`
`391`	`391`
Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,11 @@ impl Testcase {`
`92`	`92`	`}`
`93`	`93`	`}`
`94`	`94`
`95`		`- None`
	`95`	`+ if has_errors {`
	`96`	`+ Some(rv)`
	`97`	`+ } else {`
	`98`	`+ None`
	`99`	`+ }`
`96`	`100`	`}`
`97`	`101`	`}`
`98`	`102`
`@@ -138,6 +142,7 @@ fn map_tokenizer_state(input: State) -> html5gum::State {`
`138`	`142`	`State::Plaintext => html5gum::State::PlainText,`
`139`	`143`	`State::RawData(RawKind::Rcdata) => html5gum::State::RcData,`
`140`	`144`	`State::RawData(RawKind::Rawtext) => html5gum::State::RawText,`
	`145`	`+ State::RawData(RawKind::ScriptData) => html5gum::State::ScriptData,`
`141`	`146`	`x => todo!("{:?}", x),`
`142`	`147`	`}`
`143`	`148`	`}`
`@@ -170,7 +175,7 @@ fn build_test(testcase: Testcase, fname: &str, i: usize, scripting: bool) -> Tri`
`170`	`175`
`171`	`176`	`let token_emitter = Html5everEmitter::new(&mut tree_builder);`
`172`	`177`
`173`		`- let input = &testcase.data[..testcase.data.len() - 1];`
	`178`	`+ let input = &testcase.data[..testcase.data.len().saturating_sub(1)];`
`174`	`179`	`let mut tokenizer = Tokenizer::new_with_emitter(input, token_emitter);`
`175`	`180`	`if let Some(state) = initial_state {`
`176`	`181`	`tokenizer.set_state(state);`
`@@ -290,6 +295,10 @@ fn main() {`
`290`	`295`	`let args = Arguments::from_args();`
`291`	`296`	`let mut tests = Vec::new();`
`292`	`297`
	`298`	`+ for entry in glob("tests/custom-html5lib-tests/tree-construction/*.dat").unwrap() {`
	`299`	`+ produce_testcases_from_file(&mut tests, &entry.unwrap());`
	`300`	`+ }`
	`301`	`+`
`293`	`302`	`for entry in glob("tests/html5lib-tests/tree-construction/*.dat").unwrap() {`
`294`	`303`	`produce_testcases_from_file(&mut tests, &entry.unwrap());`
`295`	`304`	`}`