Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Comments for "https://os.phil-opp.com/integration-tests/" #443

Closed
utterances-bot opened this issue Jun 21, 2018 · 31 comments
Closed

Comments for "https://os.phil-opp.com/integration-tests/" #443

utterances-bot opened this issue Jun 21, 2018 · 31 comments

Comments

@utterances-bot
Copy link

utterances-bot commented Jun 21, 2018

This is a general purpose comment thread for the “Integration Tests” post.

Copy link
Contributor

Great stuff!
Why is the QEMU exit device at I/O Port 0xf4? Is it just a spare port that's available in a typical x86 machine?

@AleVul
Copy link

AleVul commented Jun 21, 2018

In Additional Test Executables and Bootimage Test paragraphs the author mentions unit test, but judging by context, aren't those supposed be integration tests?

Copy link

atedp commented Jun 23, 2018

Fist off thanks for the great tutorial!

On my machine I seem to have to append --target x86_64-blog_os.json to all bootimage commands...not a huge deal, just curious if I am doing something obviously wrong.

without the target called out specifically I get:
error: language item required, but not found: eh_personality

@phil-opp phil-opp changed the title integration-tests/ Comments for "https://os.phil-opp.com/integration-tests/" Jun 27, 2018
@phil-opp
Copy link
Owner

@AleVul Good catch, thanks! Fixed in 22470e7.

@phil-opp
Copy link
Owner

phil-opp commented Jun 27, 2018

@atedp We configured bootimage to use the x86_64-blog_os.json target as default target here. Maybe you read that post before we introduced this paragraph. Without setting the default, bootimage tries to build for your host architecture, I think that's why the error occurs.

@phil-opp
Copy link
Owner

@skierpage Thanks!

Why is the QEMU exit device at I/O Port 0xf4? Is it just a spare port that's available in a typical x86 machine?

Yeah, I think so. Here is an incomplete list of the common port ranges.

@atedp
Copy link

atedp commented Jun 27, 2018

@phil-opp Woops, I definitely missed that :)

Thanks!!

Copy link

pbn4 commented Jun 30, 2018

Hello very good tutorial. Thanks for sharing this. One question popped up though:

What is the purpose of using:

#![feature(const_fn)]

in src/main.rs?

@skierpage
Copy link
Contributor

What is the purpose of using:
#![feature(const_fn)]
in src/main.rs?

I don't see it in main.rs. It's in src/lib.rs, possibly a holdover from earlier version. https://os.phil-opp.com/vga-text-mode/#a-global-interface references const functions, but explains how it's using lazy_static instead.

@phil-opp
Copy link
Owner

phil-opp commented Jul 1, 2018

possibly a holdover from earlier version.

Seems like it. Should be removed in 3365a4f. Thanks for reporting!

Copy link

dodikk commented Jul 4, 2018

@phil-opp , thank you for another great article.

Could you please explain the reason of choosing the "Additional Test Executables" approach over "Read the test function name from the serial port" (and dispatch it to actual Rust functions with match operator or some other means. Basically, that would be a simple interpreter over a serial port).

As far as I remember, UART allows a two-way communication... (and you've also mentioned that fact in the beginning of an article).

P.S. explaining how to build a library and a "hybrid project" clearly adds some education value to the article. Still, please consider highlighting the pros and cons of different test framework implementation approaches. So far It's a bit unclear for the beginners (like me) "what the best practice is".

P.P.S. I have created a dedicated issue #451

@nukeop
Copy link

nukeop commented Jul 5, 2018

I followed the tutorial and found that lib.rs off also needs this piece of code at the top to make cargo test work:

#[cfg(test)]
extern crate std;

#[cfg(test)]
extern crate array_init;

Since this was previously in main.rs, the tests won't run without it. Or did I miss something?

Copy link

Hello again @phil-opp :D,

working again through your tutorial. I encountered one minor problem. Which i believe is just my in proper use of the tools. Nonetheless i've got a question:

the qemu commandline option "-device isa-debug-exit,iobase=0xf4,iosize=0x04"

i cannot find any concrete information about this option. could you please tell from where you got it?

Copy link
Contributor

mtn commented Jul 7, 2018

@nukeop It's included in the example code for lib.rs:

#![no_std] // don't link the Rust standard library

...

#[cfg(test)]
extern crate array_init;
#[cfg(test)]
extern crate std;

...

@phil-opp
Copy link
Owner

phil-opp commented Jul 9, 2018

@nukeop As @mtn said, it is part of the code listing in the “Split Off A Library” section.

@phil-opp
Copy link
Owner

phil-opp commented Jul 9, 2018

@Johanneslueke It is mentioned in the “Shutdown” article of the OSDev wiki and this Stackoverflow answer. The official documentation is rather sparse unfortunately. You see it under "Misc devices" when executing qemu-system-x86_64 -device help and with qemu-system-x86_64 -device isa-debug-exit,help you see the two configuration options. Other than that, I only found the commit that added it.

Copy link

ksqsf commented Jul 11, 2018

We mark the function (exit_qemu) as unsafe because it relies on the fact that a special QEMU device is attached to the I/O port with address 0xf4.

I'm not exactly sure why this would be a reason to mark exit_qemu unsafe. I think if a program exits, there will be no memory safety problems. Could you please elaborate on this? Thanks!

@phil-opp
Copy link
Owner

@ksqsf The code relies on two conditions: (1) There is a device attached at port 0xf4 and (2) this device is the QEMU shutdown device. Both conditions might not hold in a real system. In a really bad theoretical case there could a different device at that address that interprets the sent command as "overwrite some memory region via DMA", which would break memory safety. So because we cannot prove that this function is safe in all cases, we mark it as unsafe and use it only in the integration test scenario where we know that the QEMU device exists at that address.

Copy link

ghost commented Aug 12, 2018

I am getting the following error :

error: format argument must be a string literal
--> src/bin/test-panic.rs:22:5
|
22 | serial_println!("ok");
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
help: you might be missing a string literal to format with
|
2 | serial_print ! ( "{}", concat ( $ fmt , "\n" ) ) ) ; (
| ^^^^^

Though if is use serial_println!("{}","ok") instead of serial_println!("ok") it works fine

????

@phil-opp
Copy link
Owner

You might be missing a ! at the end of concat (i.e. it should be concat!(…) instead of concat(…)). The reason that it works with serial_println!("{}","ok") is that it uses a different case of the macro, which seems to use the correct concat!(…) syntax.

Copy link

mdunnegan commented Nov 23, 2018

I am having issue with your code in the "Split Off A Library" section, specifically the line:

use blog_os::println;

I am getting this error when I run cargo build

no println in the root

@phil-opp
Copy link
Owner

@mdunnegan Do you have the #[macro_export] attribute on your println macro? We recently updated the blog to the 2018 edition of Rust and changed the macro imports to normal use imports instead of #[macro_use] attributes.

In case you're using the older version in your code and don't want to update it, you can add a #[macro_use] extern crate blog_os instead of use blog_os::println.

Copy link

krsoninikhil commented Feb 21, 2019

Hi @phil-opp, thank you for this wonderful series. I'm learning about OS as well as Rust.

I've few questions/doubts if you could please help me with:

  • Since integration tests don't have to be compiled while doing unit testing (cargo test), can these test-*.rs files have #![cfg(not(test))] as inner attribute? This would eliminate the need of same check for every function.
  • Why did you not use snake_case for naming integration test files?
  • Can writing to serial port (0x3F8) and port I/O (0xf4) be done using same crate - uart_16550 or x86_64? Aren't both of them doing port-mapped I/O?

@phil-opp
Copy link
Owner

Hi @krsoninikhil,

  • Since integration tests don't have to be compiled while doing unit testing (cargo test), can these test-*.rs files have #![cfg(not(test))] as inner attribute? This would eliminate the need of same check for every function.

Yes, you could do that if you are sure that you'll never have unit tests in these files. The disadvantage of this approach is that any tests that you might add in the future will be silently ignored. A better solution in my opinion would be to change the behavior of cargo test so that no output is printed for binaries without unit tests. Such a quiet mode is discussed in rust-lang/rust#38758.

  • Why did you not use snake_case for naming integration test files?

No real reason. Seperation with - seems more common for binaries in my experience, but snake case is fine too.

  • Can writing to serial port (0x3F8) and port I/O (0xf4) be done using same crate - uart_16550 or x86_64? Aren't both of them doing port-mapped I/O?

There is a difference between serial ports and I/O ports. I/O ports are the fundamental building block that can be used to read/write device registers. The serial port controller is such an device with multiple device registers.

If you look at the source code of the uart_16550 crate, you see that the SerialPort structs contains 6 device registers, which can be individually addressed using port I/O. For performing these port I/O operations, the uart_16550 crate uses the x86_64 crate behind the scenes.

I hope this answers your questions!

@krsoninikhil
Copy link

Thanks @phil-opp for clarifying, this certainly helps.

@guidao
Copy link

guidao commented Feb 26, 2019

I am getting the following error when i run 'bootimage run' command:
os: macos
rust version: rustc 1.34.0-nightly (097c04cf4 2019-02-24)

blog_os git:(post-05)> bootimage run --bin test-basic-boot --
-serial mon:stdio -display none
-device isa-debug-exit,iobase=0xf4,iosize=0x04
Building kernel
Compiling core v0.0.0 (/Users/xxx/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libcore)
Finished release [optimized] target(s) in 26.20s
error: failed to load source for a dependency on compiler_builtins

Caused by:
Unable to update /Users/xxx/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libcompiler_builtins

Caused by:
failed to read /Users/xxx/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libcompiler_builtins/Cargo.toml

Caused by:
No such file or directory (os error 2)
error: "cargo" "rustc" "-p" "compiler_builtins" "--release" "--manifest-path" "/var/folders/w3/25y_3fds4l989bp5mlb07m4w0000gn/T/xargo.0DAHd9Da08yd/Cargo.toml" "--target" "/Users/xxx/code/rust/blog_os/x86_64-blog_os.json" "--" "--sysroot" "/Users/xxx/code/rust/blog_os/target/sysroot" "-Z" "force-unstable-if-unmarked" failed with exit code: Some(101)
note: run with RUST_BACKTRACE=1 for a backtrace

@AntoineSebert
Copy link
Contributor

Caused by:
failed to read /Users/xxx/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libcompiler_builtins/Cargo.toml

It does seem to come from the compiler itself. Maybe the rust toolchain you're using is broken ?
Have you tried another nightly build ?
You can use rustup to manage your rust toolchains.

@phil-opp
Copy link
Owner

@guidao Are you using rustup for managing your toolchains? What's the output of rustc --print sysroot for you? Do the libcore and libcompiler_builtin directories exist in /Users/xxx/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/?

@guidao
Copy link

guidao commented Feb 27, 2019

@AntoineSebert @phil-opp yes, i am using rustup to manage my toolchains.

rustc --print sysroot
/Users/xxx/.rustup/toolchains/nightly-x86_64-apple-darwin
➜ src pwd
/Users/xxx/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src
➜ src ls
build_helper libcore libpanic_unwind libprofiler_builtins librustc_lsan librustc_tsan libterm libunwind tools
liballoc libpanic_abort libproc_macro librustc_asan librustc_msan libstd libtest stdsimd

but libcompiler_builtin is not found in this directory

@phil-opp
Copy link
Owner

phil-opp commented Feb 27, 2019

@guidao Thanks! I think the problem is that your cargo-xbuild version is too old: the compiler builtins library was moved to crates.io recently so cargo xbuild shouldn't look for the compiler builtins source locally. Could you try cargo install cargo-xbuild --force to update it?

@guidao
Copy link

guidao commented Feb 27, 2019

@phil-opp it's working. thanks.

Repository owner locked and limited conversation to collaborators Jun 12, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests