Add api to run specific test #2

nlamirault · 2015-11-02T11:21:11Z

With this file :

#[test]
fn it_works() {
}

#[test]
fn it_works_another() {

}

#[test]
fn just_a_test() {

}

When pointer is in it_works_another, with M-x cargo-process-current-test, cargo is called to run specific test :

-*- mode: cargo-process; default-directory: "~/Perso/cargo.el/var/hello_world/src/" -*-
Cargo-Process started at Mon Nov  2 12:24:19

cargo test it_works_another
     Running /home/nlamirault/Perso/cargo.el/var/hello_world/target/debug/hello_world-9f660b2810b4c89b

running 1 test
test it_works_another ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

   Doc-tests hello_world

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

There is also another method to run all test from current file, but actually it doesn't work :

-*- mode: cargo-process; default-directory: "~/Perso/cargo.el/var/hello_world/src/" -*-
Cargo-Process started at Mon Nov  2 12:19:12

cargo test it_works|it_works_another|just_a_test
/usr/bin/bash: it_works_another: command not found
/usr/bin/bash: just_a_test: command not found
An unknown error occurred

To learn more, run the command again with --verbose.

Another command which fails (only 2 tests are executed) :

cargo test it_works it_works_another just_a_test
Running target/debug/hello_world-9f660b2810b4c89b

running 2 tests
test it_works ... ok
test it_works_another ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured

   Doc-tests hello_world

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

I don't really know if it is possible to launch all tests from one file with Cargo.

Signed-off-by: Nicolas Lamirault nicolas.lamirault@gmail.com

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

kwrooijen · 2015-11-02T12:12:18Z

Hey, thanks for the PR. I'll take a look at it once I'm home.

kwrooijen · 2015-11-02T13:02:48Z

cargo-process.el

+Cargo: Run the tests."
+  (interactive)
+  (let ((name (cargo-process--get-current-test)))
+    (cargo-process--start "Test" (s-concat "cargo test " name))))


Use concat instead of s-concat? Would be nice to not be dependent on 's library.

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

kwrooijen · 2015-11-02T18:37:29Z

A few things:

cargo test it_works|it_works_another|just_a_test

is failing because there are no spaces between the pipes, but this will still not work.

Taken from cargo help test:
--test NAME Test only the specified integration test

So let's say your file name is lib.rs, testing only that file should be
cargo test --test lib

Maybe you could try that instead?

kwrooijen · 2015-11-02T18:45:49Z

cargo-process.el

+      (unless (and
+               (search-backward-regexp
+                (s-concat "^[[:space:]]*fn[[:space:]]*") nil t)
+               (save-excursion (go-end-of-defun) (< start (point))))


(go-end-of-defun) is undefined, where did this come from?

A Mistake. It must be rust-end-of-defun. Defined in rust-mode [1].

[1] : https://github.com/rust-lang/rust-mode/blob/master/rust-mode.el

That's fine, then we should add rust-mode as a dependency (which isn't the case right now)

nlamirault · 2015-11-02T18:46:02Z

Great ! I will try that

kwrooijen · 2015-11-02T18:48:13Z

These are some really cool additions btw, maybe give them keybindings? e.g.
"C-c C-c C-f" For the current function
"C-c C-c C-i" For the current file

Or something else.

kwrooijen · 2015-11-02T18:50:47Z

cargo-process.el

+               (save-excursion (go-end-of-defun) (< start (point))))
+        (error "Unable to find test"))
+      (save-excursion
+        (search-forward "fn ")


Why not search-forward-regexp like the previous one? If you're going to use that regex again you should create a defconst to hold that value (instead of repeating it)

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

marcbowes · 2015-12-30T14:50:17Z

Any chance this can be merged? I'd really appreciate this functionality.

(As an aside, please crib from rspec-mode :-) That mode has been very useful for ruby development.)

kwrooijen · 2016-01-03T10:20:11Z

There are still a few things that need to be fixed, removal of the 's lib, stray comments, usage of the cargo test --test lib for file specific tests. I'm a bit busy right now but I might be able to look into these things later.

marcbowes · 2016-01-03T11:51:50Z

Thanks!

I downloaded a copy of your changes and played around with it. One thing I ran into is some projects use macros to define their tests, e.g. Cargo uses test! to get some consistent naming.

I think it'd be great if we could add arbitrary args to cargo test beyond just the automatic "run the current / all in this file" that this PR adds. For example, I added this:

;;;###autoload
(defun cargo-process-test-args (extra)
  "Run the Cargo test command with arguments.
Cargo: Run the tests with args."
  (interactive "sargs: ")
  (cargo-process--start "Test" (s-concat "cargo test " extra)))

kwrooijen · 2016-01-03T13:47:25Z

That snippet looks fine (except should use concat in favor of s-concat). Since this PR still needs a bit of fixing you could create a new PR, maybe add a shortcut key for it. Adding a custom test command would be nice since there are quite a few flags you can give.

    --lib                        Test only this package's library
    --bin NAME                   Test only the specified binary
    --example NAME               Test only the specified example
    --test NAME                  Test only the specified integration test target
    --bench NAME                 Test only the specified benchmark target
    --no-run                     Compile, but don't run tests
    -p SPEC, --package SPEC ...  Package to run tests for
    -j N, --jobs N               The number of jobs to run in parallel
    --release                    Build artifacts in release mode, with optimizations
    --features FEATURES          Space-separated list of features to also build
    --no-default-features        Do not build the `default` feature
    --target TRIPLE              Build for the target triple
    --manifest-path PATH         Path to the manifest to build tests for
    -v, --verbose                Use verbose output
    -q, --quiet                  No output printed to stdout
    --color WHEN                 Coloring: auto, always, never
    --no-fail-fast               Run all tests regardless of failure

Possibly adding auto completion for these flags in the future would be a great addition. But for now, if it benefits you, feel free to create a PR with your change. Or I can quickly add it myself since it's not too much work.

marcbowes · 2016-01-04T09:09:12Z

I'm in no hurry since I was able to make these changes locally to unblock myself.

The one thing that bothers me about this strategy is having to do it for each of the cargo commands. For example, cargo run also has a bunch of options and you might want to run examples in your IDE.

The one way to deal with this is to play keep-up. You can make or accept pull requests for changes that allow argument plumbing per cargo command. This falls flat in the face of custom cargo commands which may be public (https://github.com/kbknapp/cargo-extras) or private.

Thus, I think a better route would be to allow the top-level function (defun cargo-process--start (name command) accept optional args and then allow for each of the wrapper functions (e.g. cargo test/run) pass the optional args through either interactively or via a prefix arg.

Does that make sense?

marcbowes · 2016-01-04T09:16:24Z

(For my own education; I just hacked my snippet on top of this PR. I don't really know elisp.)

except should use concat in favor of s-concat

I did a bit of reading and it looks like these functions are identical? Is it just defined for convenience/consistency in magnars/s.el or is there some other benefit to that definition?

kwrooijen · 2016-01-04T11:31:22Z

Thus, I think a better route would be to allow the top-level function (defun cargo-process--start (name command) accept optional args and then allow for each of the wrapper functions (e.g. cargo test/run) pass the optional args through either interactively or via a prefix arg.

This could be done, we can simply add a &rest argument and concatenate the command. Would be a clean solution I think (if that's what you meant)

except should use concat in favor of s-concat

I did a bit of reading and it looks like these functions are identical? Is it just defined for convenience/consistency in magnars/s.el or is there some other benefit to that definition?

The reason is that they're identical. If we add s-concat we'll have to add 's as a dependency. This isn't a bad thing per se, but I'd like avoid unnecessary dependencies.

marcbowes · 2016-01-04T14:59:26Z

👍

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

Like that: M-x cargo-process-current-test -*- mode: cargo-process; default-directory: "~/Perso/cargo.el/var/hello_world/tests/" -*- Cargo-Process started at Tue Jan 5 10:22:12 cargo test it_works_another Running /home/nlamirault/Perso/cargo.el/var/hello_world/target/debug/hello_world-9f660b2810b4c89b running 1 test test it_works_another ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured Running /home/nlamirault/Perso/cargo.el/var/hello_world/target/debug/lib-dff9d964ea838ea8 running 1 test test it_works_another ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured Doc-tests hello_world running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured M-x cargo-process-current-file-tests -*- mode: cargo-process; default-directory: "~/Perso/cargo.el/var/hello_world/tests/" -*- Cargo-Process started at Tue Jan 5 10:19:53 cargo test --test lib Running /home/nlamirault/Perso/cargo.el/var/hello_world/target/debug/lib-dff9d964ea838ea8 running 3 tests test it_works_another ... ok test it_works ... ok test just_a_test ... ok test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

kwrooijen · 2016-01-07T12:12:27Z

cargo-process.el

+    name))
+
+
+;; (defun cargo-process--get-current-file-tests ()


This can be removed now, right? :)

kwrooijen · 2016-01-07T12:13:23Z

Looks great, thank you! Just fix those last 2 comments and I'll gladly merge :)
Edit: One last thing! Please update the readme with the new functions!

kwrooijen · 2016-01-08T23:36:19Z

I'll just merge this and add my own extra changes, had some extra time on my hands :) thanks for this! 👍

Add api to run specific test

kwrooijen · 2016-01-08T23:54:12Z

Just for clarification, here are my changes. a6e367e

nlamirault · 2016-01-09T08:59:03Z

Great

Add api to run specific test

6ded326

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

kwrooijen reviewed Nov 2, 2015
View reviewed changes

Remove checking buffer

cf84950

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

kwrooijen reviewed Nov 2, 2015
View reviewed changes

nlamirault added 2 commits November 3, 2015 09:28

Refactoring using regex to extract rust test

d9e172e

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

Add keybindings

dc29678

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

nlamirault added 2 commits January 5, 2016 10:11

Remove s.el usage

c4001e5

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>

kwrooijen reviewed Jan 7, 2016
View reviewed changes

cargo-process.el

name))

;; (defun cargo-process--get-current-file-tests ()

Copy link

Owner

kwrooijen Jan 7, 2016

Choose a reason for hiding this comment

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

This can be removed now, right? :)

kwrooijen added a commit that referenced this pull request Jan 8, 2016

Merge pull request #2 from nlamirault/develop

5edca5d

Add api to run specific test

kwrooijen merged commit 5edca5d into kwrooijen:master Jan 8, 2016

farva added a commit to farva/cargo.el that referenced this pull request Oct 30, 2018

CR fixes kwrooijen#2

b553992

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add api to run specific test #2

Add api to run specific test #2

nlamirault commented Nov 2, 2015

kwrooijen commented Nov 2, 2015

kwrooijen Nov 2, 2015

kwrooijen commented Nov 2, 2015

kwrooijen Nov 2, 2015

nlamirault Nov 3, 2015

kwrooijen Nov 3, 2015

nlamirault commented Nov 2, 2015

kwrooijen commented Nov 2, 2015

kwrooijen Nov 2, 2015

marcbowes commented Dec 30, 2015

kwrooijen commented Jan 3, 2016

marcbowes commented Jan 3, 2016

kwrooijen commented Jan 3, 2016

marcbowes commented Jan 4, 2016

marcbowes commented Jan 4, 2016

kwrooijen commented Jan 4, 2016

marcbowes commented Jan 4, 2016

kwrooijen Jan 7, 2016

kwrooijen commented Jan 7, 2016

kwrooijen commented Jan 8, 2016

kwrooijen commented Jan 8, 2016

nlamirault commented Jan 9, 2016

Add api to run specific test #2

Add api to run specific test #2

Conversation

nlamirault commented Nov 2, 2015

kwrooijen commented Nov 2, 2015

kwrooijen Nov 2, 2015

Choose a reason for hiding this comment

kwrooijen commented Nov 2, 2015

kwrooijen Nov 2, 2015

Choose a reason for hiding this comment

nlamirault Nov 3, 2015

Choose a reason for hiding this comment

kwrooijen Nov 3, 2015

Choose a reason for hiding this comment

nlamirault commented Nov 2, 2015

kwrooijen commented Nov 2, 2015

kwrooijen Nov 2, 2015

Choose a reason for hiding this comment

marcbowes commented Dec 30, 2015

kwrooijen commented Jan 3, 2016

marcbowes commented Jan 3, 2016

kwrooijen commented Jan 3, 2016

marcbowes commented Jan 4, 2016

marcbowes commented Jan 4, 2016

kwrooijen commented Jan 4, 2016

marcbowes commented Jan 4, 2016

kwrooijen Jan 7, 2016

Choose a reason for hiding this comment

kwrooijen commented Jan 7, 2016

kwrooijen commented Jan 8, 2016

kwrooijen commented Jan 8, 2016

nlamirault commented Jan 9, 2016