-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(testing): in-qemu tests #59
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,10 +22,27 @@ jobs: | |
with: | ||
command: bootimage | ||
args: --target=x86_64-mycelium.json | ||
- name: Test | ||
|
||
test: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v1 | ||
- name: install rust toolchain | ||
uses: actions-rs/toolchain@v1 | ||
with: | ||
profile: minimal | ||
toolchain: nightly | ||
components: rust-src, llvm-tools-preview | ||
- name: install dev env | ||
uses: actions-rs/[email protected] | ||
with: | ||
command: dev-env | ||
- name: install qemu | ||
run: sudo apt-get install qemu | ||
- name: run tests | ||
uses: actions-rs/[email protected] | ||
with: | ||
command: test | ||
command: test-x64 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it looks like this only runs the in-qemu kernel tests, so if we add regular Rust unit tests in any of the lib crates, they're no longer being run on CI? IMHO we should probably have separate CI jobs for the in-qemu kernel tests and for regular rust unit tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think they were running on CI before anyway, as we were only running There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
oh whoops, lol. |
||
|
||
clippy: | ||
runs-on: ubuntu-latest | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
members = [ | ||
"alloc", | ||
"util", | ||
"util-macros", | ||
"hal-core", | ||
"hal-x86_64", | ||
] | ||
|
@@ -12,12 +13,10 @@ version = "0.0.1" | |
authors = ["Eliza Weisman <[email protected]>"] | ||
edition = "2018" | ||
|
||
[lib] | ||
name = "mycelium_kernel" | ||
|
||
[dependencies] | ||
hal-core = { path = "hal-core" } | ||
mycelium-alloc = { path = "alloc" } | ||
mycelium-util = { path = "util" } | ||
tracing = { version = "0.1", default_features = false } | ||
|
||
[target.'cfg(target_arch = "x86_64")'.dependencies] | ||
|
@@ -35,6 +34,11 @@ wat = "1.0" | |
|
||
[package.metadata.bootimage] | ||
default-target = "x86_64-mycelium.json" | ||
test-success-exit-code = 33 # (0x10 << 1) | 1 | ||
test-args = [ | ||
"-device", "isa-debug-exit,iobase=0xf4,iosize=0x04", | ||
"-serial", "stdio", "-display", "none" | ||
] | ||
|
||
[package.metadata.target.'cfg(target_arch = "x86_64")'.cargo-xbuild] | ||
memcpy = true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,10 +66,9 @@ impl BootInfo for RustbootBootInfo { | |
} | ||
|
||
#[no_mangle] | ||
#[cfg(target_os = "none")] | ||
pub extern "C" fn _start(info: &'static bootinfo::BootInfo) -> ! { | ||
let bootinfo = RustbootBootInfo { inner: info }; | ||
mycelium_kernel::kernel_main(&bootinfo); | ||
crate::kernel_main(&bootinfo); | ||
} | ||
|
||
#[cold] | ||
|
@@ -122,3 +121,22 @@ pub(crate) fn oops(cause: &dyn core::fmt::Display) -> ! { | |
} | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
#[repr(u32)] | ||
pub(crate) enum QemuExitCode { | ||
Success = 0x10, | ||
Failed = 0x11, | ||
} | ||
|
||
/// Exit using `isa-debug-exit`, for use in tests | ||
/// | ||
/// NOTE: This is a temporary mechanism until we get proper shutdown implemented | ||
#[cfg(test)] | ||
pub(crate) fn qemu_exit(exit_code: QemuExitCode) { | ||
let code = exit_code as u32; | ||
unsafe { | ||
asm!("out 0xf4, eax" :: "{eax}"(code) :: "intel","volatile"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, take it or leave it: i think that everywhere else, we use AT&T syntax for inline assembly. i'm fine with either, but i think we might want to agree on a project convention for consistency...since the operand ordering is reversed it can be kind of confusing IMO There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (oh god does this mean we need to have a style guide) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can use the AT&T syntax, I'm just more used to intel syntax so tend to use it by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i’m fine with either syntax (i actually have a slight preference for intel as well, for extremely silly reasons: i think existing inline asm is at&t mostly just because most of it is about one instruction that doesn’t have any memory operands and i was too lazy to type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh another note is that since @iximeow is working on adding disassembly to kernel oopses using |
||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,121 @@ | |
#![cfg_attr(target_os = "none", feature(alloc_error_handler))] | ||
#![cfg_attr(target_os = "none", feature(asm))] | ||
#![cfg_attr(target_os = "none", feature(panic_info_message, track_caller))] | ||
#![cfg_attr(target_os = "none", feature(custom_test_frameworks))] | ||
#![cfg_attr(target_os = "none", test_runner(test_runner))] | ||
#![cfg_attr(target_os = "none", reexport_test_harness_main = "test_main")] | ||
|
||
pub mod arch; | ||
extern crate alloc; | ||
|
||
use core::fmt::Write; | ||
use hal_core::{boot::BootInfo, mem, Architecture}; | ||
use alloc::vec::Vec; | ||
|
||
mod wasm; | ||
mod arch; | ||
|
||
const HELLOWORLD_WASM: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/helloworld.wasm")); | ||
|
||
pub fn kernel_main<A>(bootinfo: &impl BootInfo<Arch = A>) -> ! | ||
where | ||
A: Architecture, | ||
{ | ||
let mut writer = bootinfo.writer(); | ||
writeln!( | ||
&mut writer, | ||
"hello from mycelium {} (on {})", | ||
env!("CARGO_PKG_VERSION"), | ||
A::NAME | ||
) | ||
.unwrap(); | ||
writeln!(&mut writer, "booting via {}", bootinfo.bootloader_name()).unwrap(); | ||
|
||
if let Some(subscriber) = bootinfo.subscriber() { | ||
tracing::dispatcher::set_global_default(subscriber).unwrap(); | ||
} | ||
|
||
let mut regions = 0; | ||
let mut free_regions = 0; | ||
let mut free_bytes = 0; | ||
|
||
{ | ||
let span = tracing::info_span!("memory map"); | ||
let _enter = span.enter(); | ||
for region in bootinfo.memory_map() { | ||
let kind = region.kind(); | ||
let size = region.size(); | ||
tracing::info!( | ||
" {:>10?} {:>15?} {:>15?} B", | ||
region.base_addr(), | ||
kind, | ||
size, | ||
); | ||
regions += 1; | ||
if region.kind() == mem::RegionKind::FREE { | ||
free_regions += 1; | ||
free_bytes += size; | ||
} | ||
} | ||
|
||
tracing::info!( | ||
"found {} memory regions, {} free regions ({} bytes)", | ||
regions, | ||
free_regions, | ||
free_bytes, | ||
); | ||
} | ||
|
||
A::init_interrupts(bootinfo); | ||
|
||
#[cfg(test)] | ||
test_main(); | ||
|
||
// if this function returns we would boot loop. Hang, instead, so the debug | ||
// output can be read. | ||
// | ||
// eventually we'll call into a kernel main loop here... | ||
#[allow(clippy::empty_loop)] | ||
loop {} | ||
} | ||
|
||
#[cfg(test)] | ||
fn test_runner(tests: &[&mycelium_util::testing::TestCase]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. take it or leave it: I feel like a large chunk of this could go in the |
||
let span = tracing::info_span!("==== running tests ====", count = tests.len()); | ||
let _enter = span.enter(); | ||
|
||
let mut fails = 0; | ||
for test in tests { | ||
let span = tracing::info_span!("running test", test.name); | ||
let _enter = span.enter(); | ||
Comment on lines
+90
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, TIOLI: it seems like this is something the test-case macro could generate? |
||
match (test.func)() { | ||
Ok(_) => tracing::info!(test.name, "TEST OK"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit/TIOLI: since |
||
Err(_) => { | ||
fails += 1; | ||
tracing::error!(test.name, "TEST FAIL"); | ||
} | ||
} | ||
} | ||
|
||
let exit_code; | ||
if fails == 0 { | ||
tracing::info!("Tests OK"); | ||
exit_code = arch::QemuExitCode::Success; | ||
} else { | ||
tracing::error!(fails, "Some Tests FAILED"); | ||
exit_code = arch::QemuExitCode::Failed; | ||
} | ||
|
||
arch::qemu_exit(exit_code); | ||
panic!("failed to exit qemu"); | ||
} | ||
|
||
#[global_allocator] | ||
pub static GLOBAL: mycelium_alloc::Alloc = mycelium_alloc::Alloc; | ||
|
||
#[alloc_error_handler] | ||
fn alloc_error(layout: core::alloc::Layout) -> ! { | ||
panic!("alloc error: {:?}", layout); | ||
} | ||
|
||
#[cfg(target_os = "none")] | ||
#[panic_handler] | ||
|
@@ -43,6 +156,28 @@ fn panic(panic: &core::panic::PanicInfo) -> ! { | |
arch::oops(&pp) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think we should add a line to #[cfg(test)]
qemu_exit(QemuExitCode::Failed); since the current oops handler will display the oops screen and then hang so the user can read the oops screen. this means that if a test fails by panicking or by triggering a CPU fault, rather than exiting and reporting that the test failed, we will hang forever (which we especially don't want in CI). if we are going to fail tests using the Rust alternatively, we could put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I wanted to do this, but didn't in the first version, as your oops handler hadn't been written yet, and I didn't want to make my changes conflict :-) |
||
} | ||
|
||
#[mycelium_util::test] | ||
fn test_alloc() { | ||
// Let's allocate something, for funsies | ||
let mut v = Vec::new(); | ||
tracing::info!(vec = ?v, vec.addr = ?v.as_ptr()); | ||
v.push(5u64); | ||
tracing::info!(vec = ?v, vec.addr = ?v.as_ptr()); | ||
v.push(10u64); | ||
tracing::info!(vec = ?v, vec.addr = ?v.as_ptr()); | ||
assert_eq!(v.pop(), Some(10)); | ||
assert_eq!(v.pop(), Some(5)); | ||
} | ||
|
||
|
||
#[mycelium_util::test] | ||
fn test_wasm() { | ||
match wasm::run_wasm(HELLOWORLD_WASM) { | ||
Ok(()) => tracing::info!("wasm test Ok!"), | ||
Err(err) => tracing::error!(?err, "wasm test Err"), | ||
} | ||
Comment on lines
+175
to
+178
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: since we already log the output of the test case in the test harness wrapper, can't this just be #[mycelium_util::test]
fn test_wasm() {
wasm::run_wasm(HELLOWORLD_WASM)
} |
||
} | ||
Comment on lines
+159
to
+179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit/TIOLI: shouldn't these go in a test module? |
||
|
||
fn main() { | ||
unsafe { | ||
core::hint::unreachable_unchecked(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
[package] | ||
name = "mycelium-util-macros" | ||
version = "0.1.0" | ||
authors = ["Nika Layzell <[email protected]>"] | ||
edition = "2018" | ||
|
||
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html | ||
[lib] | ||
proc-macro = true | ||
|
||
[dependencies] | ||
syn = { version = "1.0", features = ["full"] } | ||
proc-macro2 = "1.0" | ||
quote = "1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder if this would still fail if we changed it to