phil-opp / blog_os

Writing an OS in Rust
http://os.phil-opp.com
Apache License 2.0
14.27k stars 1.01k forks source link

Comments for "https://os.phil-opp.com/testing/" #591

Closed utterances-bot closed 3 years ago

utterances-bot commented 5 years ago

This is a general purpose comment thread for the Testing post.

leggettc18 commented 5 years ago

I'm not sure if I did something wrong, but by following along with this guide exactly I was receiving an error about fmt being undeclared after setting up the CompareMessage implementation. I fixed it by adding use core::fmt; and use core::fmt::Write; at the top of the file, and increasing the PANIC_LINE by 2 to compensate. Did I miss a step along the way or is there a missing step in the guide?

phil-opp commented 5 years ago

@leggettc18 You did nothing wrong, I forgot to mention these imports. Fixed in 1f9710308f6823df69d8a7eab582b861c2cd5c96. Thanks for reporting!

Beidah commented 5 years ago

I'm unable to run cargo xtest. Instead I keep keeping the message

error[E0463]: can't find crate for std | = note: the x86_64-blog_os-1954185736335484285 target may not be installed

cargo xbuild works fine, though.

AntoineSebert commented 5 years ago

Try cargo xtest --target x86_64-blog_os.json.

I have the same problem but couldn't figure out why the file isn't automatically found.

phil-opp commented 5 years ago

@AntoineSebert Do you have a build.target key defined in your .cargo/config file?

@Beidah Sounds like forgot a #![no_std] attribute (perhaps the conditional attribute in your lib.rs?). Or you accidentally use a dependency that uses std.

AntoineSebert commented 5 years ago

@phil-opp Yes it's there. The problem seems to have disappread today, and anyway it's not a big deal.

AntoineSebert commented 5 years ago

The recent improvements in the test framework now produce text outputs that are not colored in green, thus making the test result less visible within the terminal. Is there a way to specify a color for the text when calling print/println macros ?

phil-opp commented 5 years ago

Yes, there is! You can use ANSI escape codes for coloring. For example, you can create a type to print something in green:

use core::fmt;

struct Green(&'static str);

impl fmt::Display for Green {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { 
        write!(f, "\x1B[32m")?; // prefix code
        write!(f, "{}", self.0)?;
        write!(f, "\x1B[0m")?; // postfix code
        Ok(())
    }
}

Now you can do serial_println!("{}", Green("[ok]")); at the end of your test.

I opened https://github.com/phil-opp/blog_os/issues/603 for adding this to the post.

meteor-lsw commented 5 years ago

In the process of adding tests, I encountered some strange errors. In this test, I simply created a writer, but failed, and judging from the error message, it seems that it did not execute any tests at all, because I did not see the information printed by test_runner or panic_handler on the console.

截屏-20190514233300-1920x1052

lws@lsw:/document/project/flandre-os/FlandreOS$ cargo xtest test_default_writer
   Compiling flandre_os v0.0.1 (/document/project/flandre-os/FlandreOS)
    Finished dev [unoptimized + debuginfo] target(s) in 0.33s
     Running target/x86_64-flandre_os/debug/deps/flandre_os-d120248f92b70111
Building bootloader
   Compiling bootloader v0.6.0 (/work/tool/rust/cargo/registry/src/mirrors.ustc.edu.cn-61ef6e0cd06fb9b8/bootloader-0.6.0)
    Finished release [optimized + debuginfo] target(s) in 0.72s
Running: `qemu-system-x86_64 -drive format=raw,file=/document/project/flandre-os/FlandreOS/target/x86_64-flandre_os/debug/deps/bootimage-flandre_os-d120248f92b70111.bin -device isa-debug-exit,iobase=0xf4,iosize=0x04 -serial stdio -display none test_default_writer`
qemu-system-x86_64: -display none: drive with bus=0, unit=0 (index=0) exists
error: test failed, to rerun pass '--lib'
meteor-lsw commented 5 years ago

I did all kinds of printing tests and the printing function seemed to work normally, but when I tried to add some tests to ensure that these functions would not be damaged in future modifications, I got the above errors.

bjorn3 commented 5 years ago

I think passing a test name to cargo xtest doesnt work. Try running just cargo xtest.

phil-opp commented 5 years ago

I think passing a test name to cargo xtest doesnt work. Try running just cargo xtest.

Yes, as you see from the "Running: " output, the test_default_writer is directly passed as an argument to QEMU, which does not understand it and exits with an error. Passing the name of a single test is a feature of the default test runner and not available in our bare-metal environment because our kernel does not get any arguments passed when it's started.

What you can do is to run the tests of the library component through cargo xtest --lib and the tests of the binary through cargo xtest --bin blog_os. You can also run an individual integration test through cargo xtest --test test_name.

xj42 commented 5 years ago

Now we can run the test using cargo xtest --test panic_handler. We see that it passes, which means that the reported panic info is correct. If we use a wrong line number in PANIC_LINE or panic with an additional character through panic!("{}x", MESSAGE), we see that the test indeed fails.

I tried adding the "x" as you indicated in the above paragraph. The test did not fail. WIth some more fooling about i see that the fn write_str(&mut self, s: &str) -> fmt::Result {} function is called 4 times. The third time it is called, it sets equals to true. Because there is no else setting false, the test does not work as expected.

I have submitted a pull request #610

phil-opp commented 5 years ago

@xj42 Thanks a lot for reporting! I created https://github.com/phil-opp/blog_os/pull/611 to fix this.

ta5een commented 5 years ago

Hi, thank you for taking your time and effort into making this brilliant blog! I've learnt so much about operating systems and the Rust language in general! Can't wait to see how this blog unfolds and what new things you'll bring.

I just wanted to ask if it is the expected behaviour for serial_print! and serial_println! to only work in test mode? With cargo xtest, the serial print macros work as expected; they print to my host's terminal. However, with cargo xrun, nothing is shown in my host's terminal, nor my QEMU instance. Oddly enough, the system hasn't panicked, so it somehow worked (but the bytes don't show up).

It seems to me that perhaps this was the desired behaviour? If so, is there a way for me to send data to the serial port while the QEMU instance is running as normal?

phil-opp commented 5 years ago

@tahscenery Thanks so much for the nice words!

Yes, this is the expected behavior. The reason is that the package.metadata.bootimage.test-args configuration key in your Cargo.toml only applies to cargo xtest and is ignored for cargo xrun. So the -serial stdio argument is only added to Q.EMU in test mode. Without it, QEMU simply ignores the serial output.

To also print the serial output for cargo xrun, you can add a run-args = ["-serial", "stdio"] key to the [package.metadata.bootimage] section in your Cargo.toml. The run-args key is the counterpart to the test-args key: it applies only to cargo xrun, but not to cargo xtest. See the bootimage Readme for more information.

ta5een commented 5 years ago

@phil-opp Of course, I knew it was that simple 🤦‍♂️! Thank you so much!

AtsukiTak commented 4 years ago

Hi, thank you so much for writing this awesome blog. I'm a very fun of it!

To make our test_runner available to executables and integration tests, we don't apply the cfg(test) attribute to it and make it public.

I'm not sure why we need to remove cfg(test) attribute to make it accessible to src/main.rs on test mode. Is src/main.rs on test mode linked to src/lib.rs on non-test mode?

phil-opp commented 4 years ago

Is src/main.rs on test mode linked to src/lib.rs on non-test mode?

Yes, exactly. Only the leaf crate is compiled in test mode, dependencies are not. And the lib.rs is treated like a normal dependency.

GrayJack commented 4 years ago

I'm having trouble I did till Custom Test Frameworks But keep telling me that error[E0463]: can't find crate for test

I did exactly as said in the tutirial

phil-opp commented 4 years ago

@GrayJack Did you add the #![feature(custom_test_frameworks)] and #![test_runner(crate::test_runner)] attributes? If yes, do you have your code available somewhere so that we can take a look?

GrayJack commented 4 years ago

Yes, I did https://github.com/GrayJack/FerrousOS

phil-opp commented 4 years ago

@GrayJack Thanks! You already have a lib.rs, which we introduce later in the post. Rust tests your main.rs and lib.rs separately, so you need to specify the #![test_runner(crate::test_runner) attribute in both files. Otherwise, the default test runner will be used for your lib.rs, which results in the error you saw.

GrayJack commented 4 years ago

Thanks, now it's complaining that that there is no #[panic_handler] function on the lib part, do I have to create a panic handler inside lib.rs as well?

phil-opp commented 4 years ago

Yes, see the "Create a Library" section of the post:

Since our lib.rs is tested independently of our main.rs, we need to add a _start entry point and a panic handler when the library is compiled in test mode. By using the cfg_attr crate attribute, we conditionally enable the no_main attribute in this case.

lovesegfault commented 4 years ago

I keep getting this supremely annoying error when trying to write the panic handler test:

error[E0152]: duplicate lang item found: `panic_impl`.
  --> tests/panic_handler.rs:23:1
   |
23 | / fn panic(info: &PanicInfo) -> ! {
24 | |     check_location(info);
25 | |     check_message(info);
26 | |
...  |
29 | |     loop {}
30 | | }
   | |_^
   |
   = note: first defined in crate `daedalos`.

It seems like the panic handler defined in lib.rs "leaks" into everything that uses the lib, not sure why/how that's happening. Source is on GitHub.

Any ideas @phil-opp

phil-opp commented 4 years ago

@lovesegfault There can only be a single panic handler across all included crates. If your lib.rs already provides a panic handler, you can't override in your test binary. (You also can't override the panic handler of the standard library, which is the reason why panic::set_hook exists.)

To solve this problem, move your #[cfg(not(test))] panic handler from your lib.rs to your main.rs. Then the test binaries can (and must) define their own panic handler.

chrrs commented 4 years ago

Just some confusion that came up in my mind: By writing the panic handler test, aren't you actually testing the the correct calling with the correct info of the panic handler, instead of how the panic handler is implemented in the OS itself, since there are no references back to the OS itself, except for some testing utilities? Because the more I think about it, the more it seems to me it's a seperate program, just reusing some of the testing utilities of the OS. And if thats true, aren't we testing something that's out of our hands to fix if it does fail?

phil-opp commented 4 years ago

@SlagHoedje The main reason for that test is that I wanted to show how no-harness tests work. However, you're right that the test isn't really useful because isn't using any kernel component. Given that the test is also non-trivial due to the CompareMessage struct, I'm inclined to just remove it. Instead, the post could just show how to create a should-panic integration test.

phil-opp commented 4 years ago

@SlagHoedje I opened https://github.com/phil-opp/blog_os/pull/650 to fix this issue.

Cerber-Ursi commented 4 years ago

I'm curious, is it safe to mark exit_qemu function as diverging, or the OS code could potentially continue after it has finished (while QEMU wasn't stopped yet)?

phil-opp commented 4 years ago

@Cerberuser The problem is that it only exits if the isa-debug-exit device is available at that address. If you launch QEMU without the corresponding device parameter or run your kernel on real hardware, the exit_qemu function would have no effect.

If you want a diverging exit_qemu function, you can just add a loop {} at the end. This way, the compiler can prove that the function never returns, so that the function can be marked as diverging.

64 commented 4 years ago

@phil-opp FWIW loop {} is currently insta UB (https://github.com/rust-lang/rust/issues/28728), but with some inline asm in the middle it will work

phil-opp commented 4 years ago

@64 Yes, I'm aware of that. I hoped that this issue would be resolved by now, but it seems like I was too optimistic. We should probably work around it, for example by providing a nop function in the x86_64 crate so that we can avoid writing the inline asm ourselves.

MikailBag commented 4 years ago

To my mind, core::arch::x86_64::_mm_pause() or core::hint::spin_loop() should be OK.

AleVul commented 4 years ago

I receive the following error language item required, but not found: eh_personality when i run cargo xtest, any idea how i can fix this? using rust nightly-2019-08-09

phil-opp commented 4 years ago

@AleVul At which point in the tutorial does the error occur? Do you have your code online somewhere so that I can take a look?

AleVul commented 4 years ago

@phil-opp beginning of testing, branch here i think it's because i didn't define a panic handler when running tests.

phil-opp commented 4 years ago

@AleVul I don't see any testing code in the branch and according to github the branch is even with the master branch. Perhaps you forgot a git push?

AleVul commented 4 years ago

@phil-opp yes, you right, forgot a push, pushed now, let me know if you need anything else

phil-opp commented 4 years ago

@AleVul I think I found the problem. You don't specify the panic-strategy option in your target JSON file, which means that the core library is built with unwinding enabled. After adding the option and doing a cargo clean, it worked for me.

I updated the blog to clarify this in cd5dd17a99de776a2350a7042a445b75f38b3708. Thanks for reporting your problem!

AleVul commented 4 years ago

@phil-opp thank you for your help and great articles!

AleVul commented 4 years ago

a note here, panic strategy is set in Cargo.toml file, but only for dev and release, i guess there shoulde be for test if i stick with the Cargo.toml route

AleVul commented 4 years ago

a note here, panic strategy is set in Cargo.toml file, but only for dev and release, i guess there shoulde be for test if i stick with the Cargo.toml route

nvm, just saw update version of the post

aadit0402 commented 4 years ago

I was trying to run cargo xtest --target x86_64-blog_os.json command. but getting segmentation fault in vga_buffer.rs file. I also generated a core-dump file and did full backtrace but did not get any hint to fix this. Any leads would be appreciated. This is my github repository and added segmentation fault errors here: https://github.com/aadit0402/self-learning-os-rust/commit/4e5ce69dbb8dbe44ed7a77918531029ae8fbe30c

Thanks.

phil-opp commented 4 years ago

@aadit0402 You need to specify a runner in a .cargo/config file as described here. Otherwise, cargo xtest tries to run your kernel as a normal program, which is likely the reason for the segmanation fault.

lielongxingkong commented 4 years ago

A little mistake in sample code at No Harness Tests. There is no import of serial_print function but used below and it causes a compiling error :)

phil-opp commented 4 years ago

@lielongxingkong Thanks for reporting! Fixed in 9877e4c84d39e6f5d3ec30c10a5120302561abfc.

jb3 commented 4 years ago

I'm absolutely stumped, I've been trying to get testing to work for an hour or so now to no avail.

I am at the end of the section Exiting QEMU and I'm trying to get the xtest to exit cleanly if QemuExitCode is set to Success and error if not but I get the following going on:

joseph@Josephs-MacBook-Air freja % cargo xtest 
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running target/x86_64-freja/debug/deps/freja-c796d9a42d3ed5ec
Building bootloader
    Finished release [optimized + debuginfo] target(s) in 0.04s
Running: `qemu-system-x86_64 -drive format=raw,file=/Users/joseph/freja/target/x86_64-freja/debug/deps/bootimage-freja-c796d9a42d3ed5ec.bin -device isa-debug-exit,iobase=0xf4,iosize=0x04`
error: test failed, to rerun pass '--bin freja'

The exit code of this is 33:

joseph@Josephs-MacBook-Air freja % echo $?
33

Which should be being mapped to 0 by the config:

[package.metadata.bootimage]
test-args = ["-device", "isa-debug-exit,iobase=0xf4,iosize=0x04"]
test-success-exit-code = 33

but is not, any idea what is going on?

Also I get the following error when I try run the OS and have it trigger an ISA exit:

$ qemu-system-x86_64  -device isa-debug-exit,iobase=0xf4,iosize=0x04 -drive format=raw,file=target/x86_64-freja/debug/bootimage-freja.bin
make: *** [run] Illegal instruction: 4

I'm on macOS Catalina if this makes a difference.

phil-opp commented 4 years ago

@jos-b Sorry to hear that you have problems with the post! Do you have your code online somewhere so that I can take a look?