M-Labs/nac3
M-Labs
/
nac3
8
0
Fork 10
Code Issues 99 Pull Requests 21 Releases Wiki Activity

WIP: Run standalone demos as part of Cargo Test #356

Draft
derppening wants to merge 2 commits from enhance/cargo-test-standalone-demos into master
pull from: enhance/cargo-test-standalone-demos
merge into: M-Labs:master
Conversation 3 Commits 2 Files Changed 3 +152 -3
derppening commented 2023-11-09 15:38:38 +08:00
Collaborator
Copy Link

Not something that is particularly necessary, but this allows all tests within NAC3 (excluding nac3artiq tests) to be executed via cargo test, simplifying the testing workflow.

Not something that is particularly necessary, but this allows all tests within NAC3 (excluding `nac3artiq` tests) to be executed via `cargo test`, simplifying the testing workflow.
derppening self-assigned this 2023-11-09 15:38:38 +08:00
sb10q commented 2023-11-09 18:47:27 +08:00
Owner
Copy Link

What's going to happen to check_demos.sh?

Also I would solve this the other way i.e. run cargo test from a shell script that runs all tests. Cargo is inflexible and annoying (we use it only because there is currently no other existing solution to solve and download crates.io dependencies, not because it is any good), and Rust isn't a great scripting language. Bash/Make is superior to both in many respects for build/test systems.

What's going to happen to check_demos.sh? Also I would solve this the other way i.e. run cargo test from a shell script that runs all tests. Cargo is inflexible and annoying (we use it only because there is currently no other existing solution to solve and download crates.io dependencies, not because it is any good), and Rust isn't a great scripting language. Bash/Make is superior to both in many respects for build/test systems.
derppening commented 2023-11-10 10:44:50 +08:00
Author
Collaborator
Copy Link

What's going to happen to check_demos.sh?

I am planning to keep it for now, it doesn't really introduce any bloat to the project.

Also I would solve this the other way i.e. run cargo test from a shell script that runs all tests.

I just find it to be more idiomatic to write test cases (including integration tests) using a (or the) testing framework provided by the language, as (1) it provides a one-stop shop to auto-discover and run test cases, which is especially useful in a multi-module project like this, (2) it generally provides more ways to manage temp resources (e.g. automatically removing temp files), and (3) it can run all tests before providing a summary, something that check_demos.sh currently does not do and will likely require some refactoring to add the ability.

> What's going to happen to check_demos.sh? I am planning to keep it for now, it doesn't really introduce any bloat to the project. > Also I would solve this the other way i.e. run cargo test from a shell script that runs all tests. I just find it to be more idiomatic to write test cases (including integration tests) using a (or the) testing framework provided by the language, as (1) it provides a one-stop shop to auto-discover and run test cases, which is especially useful in a multi-module project like this, (2) it generally provides more ways to manage temp resources (e.g. automatically removing temp files), and (3) it can run all tests before providing a summary, something that `check_demos.sh` currently does not do and will likely require some refactoring to add the ability.
👍 1
derppening force-pushed enhance/cargo-test-standalone-demos from 2e44e34b52 to 0d9db5525c 2023-11-16 14:17:21 +08:00 Compare
sb10q commented 2023-12-06 10:00:49 +08:00
Owner
Copy Link

As discussed during yesterday's meeting, moving everything to cargo test + cargo run and removing the shell scripts would have clear practical advantages:

  • Solves issues with release vs. debug build.
  • Tests on Windows could be run with Wine (annoyingly, MSYS2 Bash crashes Wine).
As discussed during yesterday's meeting, moving everything to ``cargo test`` + ``cargo run`` and removing the shell scripts would have clear practical advantages: * Solves issues with release vs. debug build. * Tests on Windows could be run with Wine (annoyingly, MSYS2 Bash crashes Wine).
derppening force-pushed enhance/cargo-test-standalone-demos from 0d9db5525c to 2244f347c3 2023-12-11 15:17:38 +08:00 Compare
sb10q reviewed 2023-12-12 15:38:25 +08:00
flake.nix
@ -32,3 +32,3 @@
echo "Checking nac3standalone demos..."
echo "Running Cargo tests..."
pushd nac3standalone/demo
patchShebangs .
sb10q commented 2023-12-12 15:38:25 +08:00
Owner
Copy Link

Just remove pushd/popd and do patchShebangs nac3standalone/demo?

But I thought the plan was to remove shell scripts anyway. Then there are no shebangs to patch?

Just remove pushd/popd and do ``patchShebangs nac3standalone/demo``? But I thought the plan was to remove shell scripts anyway. Then there are no shebangs to patch?
derppening commented 2023-12-12 15:39:29 +08:00
Author
Collaborator
Copy Link

Yeah it's not ready yet, that push was just a rebase.

Marked WIP now for clarity.

Yeah it's not ready yet, that push was just a rebase. Marked WIP now for clarity.
derppening changed title from Run standalone demos as part of Cargo Test to WIP: Run standalone demos as part of Cargo Test 2023-12-12 15:39:12 +08:00
sb10q reviewed 2023-12-12 15:40:28 +08:00
nac3standalone/src/test.rs Outdated
@ -0,0 +80,4 @@
fn run_demo(file: &Path, nac3_args: &[&str]) -> io::Result<Output> {
let _lk = RUN_EXEC_MTX.lock();
Command::new("./run_demo.sh")
sb10q commented 2023-12-12 15:40:28 +08:00
Owner
Copy Link

Rewrite it in Rust
Otherwise the benefits I mentioned aren't there.

![Rewrite it in Rust](https://ih1.redbubble.net/image.4406644318.0421/st,small,507x507-pad,600x600,f8f8f8.jpg) Otherwise the benefits I mentioned aren't there.
derppening force-pushed enhance/cargo-test-standalone-demos from 2244f347c3 to 710cfd7bb9 2023-12-12 16:36:12 +08:00 Compare
derppening force-pushed enhance/cargo-test-standalone-demos from 710cfd7bb9 to affa19e88d 2023-12-12 19:56:37 +08:00 Compare
This pull request has changes conflicting with the target branch.
  • flake.nix
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b enhance/cargo-test-standalone-demos master
git pull origin enhance/cargo-test-standalone-demos

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff enhance/cargo-test-standalone-demos
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
future-extension

   
high-priority

   
low-priority

   
UROP0622
No Label
future-extension
high-priority
low-priority
UROP0622
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
derppening
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/nac3#356
No description provided.
Delete Branch "enhance/cargo-test-standalone-demos"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?