✅ Resurrect core integration tests #472
Conversation
Codecov ReportAttention: Patch coverage is
... and 27 files with indirect coverage changes 🚨 Try these New Features:
|
There was a problem hiding this comment.
I gave this a read through, thanks for getting them more up-to-speed with the current API and long backlog of changes which happened since I removed them from the workspace!
I think the tests could be expanded and better-written. I tried to translate some other old tests but it seems the core API has changed enough that it wasn't obvious how they should be re-written.
I totally agree, IIRC the scanner + job system itself has had 1-2 rewrites since these tests were written. Happy to collaborate as needed to think through what the best system moving forward might be 🙂
core/package.json
Outdated
| @@ -10,7 +10,7 @@ | |||
| "studio": "yarn dlx prisma studio", | |||
| "build": "cargo build --release", | |||
| "format": "cargo fmt --package stump_core", | |||
| "integration-tests": "cargo integration-tests", | |||
| "run-tests": "cargo test", | |||
There was a problem hiding this comment.
I think we can just remove this tbh, thanks for updating it though!
core/tests/scanner_tests.rs
Outdated
| /// TODO - something less heinous, there must be a way to wait on a job | ||
| tokio::time::sleep(Duration::from_secs(1)).await; |
There was a problem hiding this comment.
Haha I know we chatted on Discord about this, but to reiterate here I think tokio::select!ing on the event bus and a reasonable timeout is the way to go here
| .await | ||
| .unwrap(); | ||
|
|
||
| assert_ne!(items.len(), 0); |
There was a problem hiding this comment.
We can probably assert the actual expected number of items, since we know(?) how many should be scanned when we create the test library
| let migration_res = core.run_migrations().await; | ||
| assert!( | ||
| migration_res.is_ok(), | ||
| "Failed to run migrations: {:?}", | ||
| migration_res | ||
| ); |
There was a problem hiding this comment.
Just a note that IIRC this won't actually run migrations in debug/test. I think it will just push onto the DB. I'd have to double check to be sure. Either way, it is needed
JMicheli
force-pushed
the
jm/integration-tests
branch
from
c950bb3
to
94d99ec
Compare
JMicheli
force-pushed
the
jm/integration-tests
branch
from
94d99ec
to
200b89c
Compare
I'm starting this as a draft because I think there's work to do, but I want to see codecov's results for it.
This moves the contents of the old, unused core/integration-tests to the normal path for integration tests
core/testsand adds a couple of actual integration tests.I think the tests could be expanded and better-written. I tried to translate some other old tests but it seems the core API has changed enough that it wasn't obvious how they should be re-written.