Skip to content
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

Make projectile--find-matching-test Use src-dir/test-dir Properties #1734

Merged
merged 4 commits into from
Dec 20, 2021

Conversation

LaurenceWarne
Copy link
Contributor

Hi! This PR is in response to a discussion in #1650 (comment), basically it's a fix for the src-dir and test-dir properties being prematurely circumvented if they are set to strings when switching from/to test/impl files. Essentially the function for finding a test/impl file using src-dir and test-dir (set as strings) was called in projectile--find-matching-test, but a fallback already existed in projectile-find-implementation-or-test which was being used in preference.

This PR hopes to fix and simplify the above by making impl/test switching wholly contained in projectile--find-matching-file/projectile--find-matching-test.

I've also updated the doc tweaking the src-dir/test-dir as a function example since I think that situation is better handled by setting src-dir/test-dir to strings (as pointed out by the issue author in the link).

Thanks!


Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)

Thanks!

(list test-file)
(when-let ((predicate (projectile--impl-to-test-predicate impl-file)))
(projectile--best-or-all-candidates-based-on-parents-dirs
impl-file (cl-remove-if-not predicate (projectile-current-project-files))))))))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically the main change (and in projectile--find-matching-file).

(:test-suffix "Spec" :test-dir "test" :src-dir "main")
(expect (projectile--find-matching-test
"project/src/main/scala/bar/package.scala")
:to-equal '("src/test/scala/bar/packageSpec.scala")))))
Copy link
Contributor Author

@LaurenceWarne LaurenceWarne Dec 13, 2021

Choose a reason for hiding this comment

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

Test for the example in #1650. This would fail before this change, since

(when-let ((predicate (projectile--impl-to-test-predicate impl-file)))
would take precedence over using the src-dir/test-dir properties.

@LaurenceWarne
Copy link
Contributor Author

LaurenceWarne commented Dec 13, 2021

Ah earlier Emacs version compat caught me out again 🤦, back to 🔨

Edit: Looks good now, I can squash the commits also if you like.

@bbatsov bbatsov merged commit f7eba5e into bbatsov:master Dec 20, 2021
@bbatsov
Copy link
Owner

bbatsov commented Dec 20, 2021

Great PR! Thanks! 🙇‍♂️

@LaurenceWarne
Copy link
Contributor Author

Thanks!

@dklimov-allstreet
Copy link

dklimov-allstreet commented Mar 24, 2022

it looks like something is broken

(let* ((error-msg (format
                   "No matching test file found for project type `%s'"
                   (projectile-project-type)))
       (test-file (or (projectile-find-matching-test file-name)
                      (error error-msg))) ; <- bam!
       (expanded-test-file (projectile-expand-root test-file)))
  (cond ((file-exists-p expanded-test-file) expanded-test-file) ; we never get here if there's no test file 
        (projectile-create-missing-test-files 
         (projectile--create-directories-for expanded-test-file)
         expanded-test-file)
        (t (progn (error error-msg)))))

ever tried to test the code with projectile-create-missing-test-files enabled?

CC @bbatsov

@bbatsov
Copy link
Owner

bbatsov commented Mar 24, 2022

@dklimov-allstreet feel free to open a new issue or just send a PR addressing this. Looking at the problem the fix should be relatively simple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants