-
Notifications
You must be signed in to change notification settings - Fork 26
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
How to run with docker #28
Comments
@vinibispo - I've made it possible to have a custom rspec command in the latest commit. Please let me know if this works |
@vinibispo Have you been able to use |
I'll test it, sorry for the delay @olimorris |
Yeah my hunch is that absolute paths are going to be the killer here... |
@olimorris I did the following test:
I have this return: I also did the following test:
I have this return: Do you have any other test ideas? |
@evertonlopesc |
@pBorak I read this comment and also your issues on neotest. @olimorris ok, I got it Thanks for your attention! |
Hey, all! Short term, my solution is to use a shell script as my rspec executable, and mutate the paths as needed in that script. Example below. You'll obviously need to substitute your own particulars. #!/bin/bash
# Strip "/home/mmirus/git/repo/path/" from test paths sent to container
args="$(echo "$@" | sed 's/\/home\/mmirus\/git\/repo\/path\///g')"
docker compose exec web bundle exec rspec $args
Then you just do this: require("neotest").setup({
adapters = {
require("neotest-rspec")({
rspec_cmd = "my-script.sh",
}),
},
})
This is basically the formula I've used in the past with other plugins and editors that had trouble with running tests or tools in a container. Tell them to run a shell script as the executable, do any arg transformations needed in the script, then call the container with the final result. It seems to work here, but it's barely tested. Thanks for this adapter, Oli! |
@mmirus |
I did see that too. I haven't had time to look into it yet. The most obvious explanation is that something about the output from docker (or the script) is not as expected. Perhaps we can capture "normal" output and output from the script and compare. |
Good suggestion! I'll do that and see if they have any ideas. |
FWIW, this version of the script might be a slightly more correct way of handling the arg transformation, but it doesn't seem like it makes any practical difference. 🤷♂️ #!/bin/bash
# Strip "/home/mmirus/git/repo/path/" from test paths sent to container
args=("${@/\/home\/mmirus\/git\/repo\/path\//}")
docker compose exec web bundle exec rspec "${args[@]}"
|
Some progress over on the neotest ticket. nvim-neotest/neotest#89 (comment) @olimorris Does any of this spark ideas for how a more shippable solution could be constructed? |
@mmirus Tested out your 2nd solution. Works wonderfully! edit: not sure if it's problem with the workaround or the edit 2: Just learned about |
Nice work! This is super exciting. I am more than happy to accept a PR (looks like you've done all the hard work already) to change how we call Neotest based on RSpec being run inside a Docker container. We could always pass a config flag or have a global variable. |
@olimorris I think the approach outlined in my latest comment is the one I prefer of the two I suggested in my comments over there. If you agree, then no change is needed to the neotest-rspec code, unless you want to add a note to the README about how to set up the adapter to work with docker. |
@mmirus sorry I didn't see it. Whilst there's not direct support for it in neotest yet, I'd prefer to add it as a note in the readme in the usage section if you'd like to PR it |
No need to apologize! I posted it after your last comment here. I'll send up a PR for the README sometime soon. |
Big shoutout to @ramblex for the latest PR which adds support for Docker |
Hey, incredible work on this repository, and I think it would be better if we could run with docker, too, because the vast majority of companies use docker nowadays.
I thought like we could run this, setting custom_command as docker
Like:
The text was updated successfully, but these errors were encountered: