Skip to content
This repository was archived by the owner on Jan 13, 2026. It is now read-only.

Rust binary CI + small unit tests#317

Merged
julgmz merged 18 commits intodevfrom
native-tests
Oct 17, 2025
Merged

Rust binary CI + small unit tests#317
julgmz merged 18 commits intodevfrom
native-tests

Conversation

@julgmz
Copy link
Copy Markdown
Collaborator

@julgmz julgmz commented Oct 7, 2025

Extensive unit tests are largely going to be more difficult to implement. These top level ones test non-OS specific calls / business logic. Once we get into OS specific calls, we'd either have to set up a dry-run + mocking conditional to our hot paths to test logic, and/or have our runners be different OS so that they can run the tests for the OS specific logic. Something to tackle in a follow-up ticket

@@ -0,0 +1,15 @@
[workspace]
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this helps for organizing running tests better. can also be used in the future to optimize compilation on mac, but dont wanna mess with that right now

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we ended up getting improved compilation on mac locally for free with these changes

mod tests {
use super::*;

#[test]
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot of this was vibecoded based on the patterns above just to prevent drift via unit tests

Copy link
Copy Markdown
Collaborator

@evanmarshall evanmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly questions but I'd like to review it again before merging.

}))
nativeBinaries.map(binary => {
// Map electron-builder arch to Cargo target triple
const cargoArch = '${arch}' === 'arm64' ? 'aarch64' : 'x86_64'
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this lets up clean up the logic in build-binaries where we move resources around to different folders.

the removed logic was causing issues with local workspace development with resources not being where they were expected on re-compilation

.send(command)
.expect("Failed to send command to processor");
}
for l in stdin.lock().lines().map_while(Result::ok) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clippy recommended this changes, binaries were still functioning the same

Copy link
Copy Markdown
Collaborator

@evanmarshall evanmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great :shipit:

@julgmz julgmz merged commit 283249c into dev Oct 17, 2025
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants