Add pytest-source-root-dir variable

[billiams]

For #35.

Adds a pytest-source-root-dir variable that defaults to nil. When this variable is set, the test paths passed to pytest will be relative to the directory defined by that variable.

[ionrock]

I like the idea behind this change but I think it should be reworked a bit. It sounds like what we really want is to avoid the cd WORKING-DIRECTORY aspect based on some config and then allow relative paths based on another config. I'd consider two config options that avoids the initial cd portion and then an allow relative paths config item to use or consider an alternate base.

Let me know if that makes sense! Thanks for looking at this!

[billiams]

@ionrock Thanks for the response! The cd WORKING-DIRECTORY actually works fine for me since in my case, my docker-compose.yml file is in my project root directory. I can see how you might want to be able to customize that though if your docker-compose.yml file is in some project sub-directory.

Is that what you're suggesting I add?

[ionrock]

Ah, I see what you're saying. I think in that case, I might rename the config around use-relative-paths or something in that vein rather than using an alternate base that effectively seems to just force using a relative path. Let me know if I've misunderstood things!

[billiams]

I was struggling with naming this config variable. I think use-relative-paths is better but it feels like it should be a boolean setting. What about relative-test-paths-base-dir?

[billiams]

@ionrock and just to be super clear, this is my setup that uses the feature in this PR:
.dir-locals.el:

;;; Directory Local Variables
;;; For more information see (info "(emacs) Directory Variables")

((python-mode
  (pytest-global-name . "docker-compose run --rm app pytest")
  (pytest-cmd-flags . "")
  (pytest-source-root-dir . "/path/to/docker/mount/volume/")

so that instead of the default command which does not take into account the container directory structure:

cd /path/to/project/root && docker-compose run --rm app pytest /path/to/docker/mount/volume/relative/path/to/test.py

I'd like the command to be

cd /path/to/project/root && docker-compose run --rm app pytest relative/path/to/test.py

[ionrock]

Here I think we should keep this variable. Let me know what you think of this example.

My code is in ~/Projects/myapp and I mount things in docker under /opt/myapp. If I have pytest-source-root-dir (which could probably be named a little clearer but I don't have a better idea now) that means I can say I want replace the WORKING-DIRECTORY with the pytest-source-root-dir when figuring out the relative path.

For example the /project/root/path/to/test.py using a /opt/myapp would replace the /project/root with /opt/myapp. An alternative is that you use can allow a pytest-use-relative-test-paths boolean that does the behavior you're defining with your existing code.

Let me know if that makes sense and handles your use case clearly!

[billiams]

If we're just solving the running-individual-tests-in-docker problem, then my preference would be to provide one simple solution to fixing test paths in the test environment when they don't match up with the absolute path in the local environment. For my use-case, I have no need to customize the local project root, but I could see how someone might want to do that (i.e. the case where docker-compose config is not at the project root). However, I wanted to avoid over-complicating the changes I'm making to this project unless users explicitly needed that additional functionality.

My understanding of pytest is that passing in a relative test path defined from the current working directory is equivalent to passing in a correct absolute path. Since that is the case, it is more durable to pass in the relative path, since any changes to your container's working directory would necessitate a change to any hardcoded container base path if you are setting an absolute path.

I propose keeping it simple and sticking with the current setup of one additional optional string variable, which I should rename to something that makes more sense to you. Do you agree?

[ionrock]

Based on my comments above, this may need to adjust slightly to consider the boolean and what root to use when finding the relative path.

[ionrock]

@billiams I added a few comments to clarify what I'm thinking about. Let me know what you think or if you need help with the implementation. It has been a while since I did much elisp but I'm happy to dust off my scratch buffer to help out :)

[billiams]

@billiams I added a few comments to clarify what I'm thinking about. Let me know what you think or if you need help with the implementation. It has been a while since I did much elisp but I'm happy to dust off my scratch buffer to help out :)

@ionrock I really appreciate all your feedback and consideration of the changes I'm proposing. I responded to your comments, LMK if you'd like to discuss further and will definitely take you up on the elisp help if needed since I am by no means an expert.

[billiams]

@ionrock LMK if you'd like me to make any other changes to this. Thanks!