Address review comments

- Switch to single-threaded test and fix thread-local hashmap issue.
- Use apt loop instead of `loop {}` , and wait for vblank.
- Remove extraneous pthread stub.

ctru-rs/src/lib.rs

@@ -2,7 +2,7 @@
 #![crate_name = "ctru"]
 #![feature(test)]
 #![feature(custom_test_frameworks)]
-#![test_runner(test_runner::test_runner)]
+#![test_runner(test_runner::run)]
 
 /// Call this somewhere to force Rust to link some required crates
 /// This is also a setup for some crate integration only available at runtime

ctru-rs/src/services/ps.rs

@@ -94,37 +94,48 @@ mod tests {
     fn construct_hash_map() {
         let _ps = Ps::init().unwrap();
 
-        let mut m: HashMap<i32, String> = HashMap::from_iter([
+        let mut input = vec![
             (1_i32, String::from("123")),
             (2, String::from("2")),
             (6, String::from("six")),
-        ]);
+        ];
 
-        println!("{:?}", m);
+        let map: HashMap<i32, String> = HashMap::from_iter(input.clone());
 
-        m.remove(&2);
-        m.insert(5, "ok".into());
+        let mut actual: Vec<_> = map.into_iter().collect();
+        input.sort();
+        actual.sort();
 
-        println!("{:#?}", m);
+        assert_eq!(input, actual);
     }
 
     #[test]
-    #[should_panic]
     fn construct_hash_map_no_rand() {
         // Without initializing PS, we can't use `libc::getrandom` and constructing
         // a HashMap panics at runtime.
-
-        let mut m: HashMap<i32, String> = HashMap::from_iter([
+        //
+        // If any test case successfully creates a HashMap before this test,
+        // the thread-local RandomState in std will be initialized. We spawn
+        // a new thread to actually create the hash map, since even in multi-threaded
+        // test environment there's a chance this test wouldn't panic because
+        // some other test case ran before it.
+        //
+        // One downside of this approach is that the panic handler for the panicking
+        // thread prints to the console, which is not captured by the default test
+        // harness and prints even when the test passes.
+        crate::thread::Builder::new()
+            .stack_size(0x20_0000)
+            .spawn(|| {
+                let map: HashMap<i32, String> = HashMap::from_iter([
                     (1_i32, String::from("123")),
                     (2, String::from("2")),
                     (6, String::from("six")),
                 ]);
 
-        println!("{:?}", m);
-
-        m.remove(&2);
-        m.insert(5, "ok".into());
-
-        println!("{:#?}", m);
+                dbg!(map);
+            })
+            .unwrap()
+            .join()
+            .expect_err("should have panicked");
     }
 }

ctru-rs/src/test_runner.rs

@@ -9,15 +9,17 @@ use test::{ColorConfig, Options, OutputFormat, RunIgnored, TestDescAndFn, TestFn
 use crate::console::Console;
 use crate::gfx::Gfx;
 use crate::services::hid::{Hid, KeyPad};
+use crate::services::Apt;
 
 /// A custom runner to be used with `#[test_runner]`. This simple implementation
 /// runs all tests in series, "failing" on the first one to panic (really, the
 /// panic is just treated the same as any normal application panic).
-pub(crate) fn test_runner(tests: &[&TestDescAndFn]) {
+pub(crate) fn run(tests: &[&TestDescAndFn]) {
     crate::init();
 
     let gfx = Gfx::default();
     let hid = Hid::init().unwrap();
+    let apt = Apt::init().unwrap();
 
     let mut top_screen = gfx.top_screen.borrow_mut();
     top_screen.set_wide_mode(true);
@@ -38,18 +40,17 @@ pub(crate) fn test_runner(tests: &[&TestDescAndFn]) {
         exclude_should_panic: false,
         run_ignored: RunIgnored::No,
         run_tests: true,
-        // Benchmarks are not supported
+        // Don't run benchmarks. We may want to create a separate runner for them in the future
         bench_benchmarks: false,
         logfile: None,
         nocapture: false,
         // TODO: color doesn't work because of TERM/TERMINFO.
         // With RomFS we might be able to fake this out nicely...
-        color: ColorConfig::AlwaysColor,
+        color: ColorConfig::AutoColor,
         format: OutputFormat::Pretty,
         shuffle: false,
         shuffle_seed: None,
-        // tweak values? This seems to work out of the box
-        test_threads: Some(3),
+        test_threads: None,
         skip: Vec::new(),
         time_options: None,
         options: Options::new(),
@@ -61,19 +62,19 @@ pub(crate) fn test_runner(tests: &[&TestDescAndFn]) {
     // Make sure the user can actually see the results before we exit
     println!("Press START to exit.");
 
+    while apt.main_loop() {
         gfx.flush_buffers();
         gfx.swap_buffers();
+        gfx.wait_for_vblank();
 
-    loop {
         hid.scan_input();
-
         if hid.keys_down().contains(KeyPad::KEY_START) {
             break;
         }
     }
 }
 
-/// Adapted from [`test::test_main_static`], along with [`make_owned_test`].
+/// Adapted from [`test::test_main_static`] and [`test::make_owned_test`].
 fn run_static_tests(opts: &TestOpts, tests: &[&TestDescAndFn]) -> io::Result<bool> {
     let tests = tests.iter().map(make_owned_test).collect();
     test::run_tests_console(opts, tests)
@@ -98,8 +99,7 @@ fn make_owned_test(test: &&TestDescAndFn) -> TestDescAndFn {
 }
 
 /// The following functions are stubs needed to link the test library,
-/// but do nothing because we don't actually need them for it to work (hopefully).
-// TODO: move to linker-fix-3ds ?
+/// but do nothing because we don't actually need them for the runner to work.
 mod link_fix {
     #[no_mangle]
     extern "C" fn execvp(
@@ -114,15 +114,6 @@ mod link_fix {
         -1
     }
 
-    #[no_mangle]
-    extern "C" fn pthread_sigmask(
-        _how: ::libc::c_int,
-        _set: *const libc::sigset_t,
-        _oldset: *mut libc::sigset_t,
-    ) -> ::libc::c_int {
-        -1
-    }
-
     #[no_mangle]
     extern "C" fn sigemptyset(_arg1: *mut libc::sigset_t) -> ::libc::c_int {
         -1