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

feat(goto): Navigate between test and implementation #53

Merged
merged 5 commits into from
Aug 19, 2020

Conversation

olivernybroe
Copy link
Collaborator

@olivernybroe olivernybroe commented Aug 17, 2020

This PR adds support for navigating between a test and it's implementation.
image

  • Added or updated tests
  • Updated CHANGELOG.md

issues: #34

@olivernybroe olivernybroe linked an issue Aug 17, 2020 that may be closed by this pull request
@olivernybroe olivernybroe force-pushed the feat-test-finder branch 2 times, most recently from 9a534ec to ac7c6d3 Compare August 17, 2020 13:13
@olivernybroe
Copy link
Collaborator Author

Hi @BertvanHoekelen, could I get you to test out if this works and satisfies your issue?

install it manually using
Preferences > Plugins > ⚙️ > Install plugin from disk...
pest-intellij-0.4.0-alpha.1.zip

@olivernybroe
Copy link
Collaborator Author

Hey @adelf, could I get you to review the indexer? First time creating one, so would like to know if I made any huge mistakes 😄

@adelf
Copy link
Contributor

adelf commented Aug 17, 2020

Hi, @olivernybroe . This index is too greedy. It will try to find pest test functions inside every php file during indexing. Better to add at least some filter, for example file path should include "/tests/" part... it can be implemented in InputFilter. I can add it later today.

@olivernybroe
Copy link
Collaborator Author

@adelf Ah nice. Yes that makes a lot of sense. So getting directories marked as test and only look at them?

Looking a little on how isPestTestFile works, I also think we can improve that one. The code looks like this for it

fun PsiFile.isPestTestFile(): Boolean {
    if (this !is PhpFile) return false

    return PsiTreeUtil.findChildrenOfType(this, FunctionReferenceImpl::class.java)
        .any(FunctionReferenceImpl::isPestTestFunction)
}

So it basically fetches all FunctionReferences and then checks if one of them is a pest test functions. I was thinking if we could somehow check this with an iterator/tree traverser so it on each FunctionReference checks if they are a pest function and stops if that is true. This way it won't have to traverse the whole tree first and then check each element, but can stop earlier in checking the tree, if it hits a pest test early on. Just now sure how to implement that 😟

@adelf
Copy link
Contributor

adelf commented Aug 17, 2020

Fixed. Index tests shouldn't try to find a virtual file in the list. Just the key value in the key list.

@olivernybroe
Copy link
Collaborator Author

olivernybroe commented Aug 18, 2020

@adelf I updated InputFilter so it should limit based on test directory.

It looks if the path contains any test folders, e.g. so it can match

  • /test/.../MyTest.php
  • /tests/.../MyTest.php
  • /src/test/.../MyTest.php

It then also checks all test source folders, so if users have some special setup where the test folder is not called test, but is marked as test, it still works.

Do you see any issues with this?

@BertvanHoekelen
Copy link

Hi @olivernybroe, I have just installed the alpha and it works like a charm 🎉 Thanks!

@olivernybroe
Copy link
Collaborator Author

@BertvanHoekelen Awesome! Thanks for testing it out.

We will fix some performance issues with it and then I think we will release a new version of the plugin where this feature will be part of it.

@adelf
Copy link
Contributor

adelf commented Aug 18, 2020

https://github.com/pestphp/pest-intellij/blob/master/src/main/kotlin/com/pestphp/pest/PestTestFileUtil.kt#L36 here I find all beforeEach calls, assuming they are in the top of the file. So:

beforeEach(...)

Will be found, but

if (...) {
    beforeEach(...)
}

this one won't. So, I check only first level of the tree, which of course much faster. If it's ok, we can use the same for isPestTestFile method.

@olivernybroe
Copy link
Collaborator Author

@adelf Ah that's brilliant!

Should help a lot with indexing on big files which are not pest tests.

If people have tests which are wrapped in if statements of something like that, then I am okay with not supporting it.

@olivernybroe
Copy link
Collaborator Author

olivernybroe commented Aug 19, 2020

@adelf I'll merge this in and make a EAP release for 0.4.0-alpha.2.

I think i'll make the 0.4.0 release when you have finished up what you are working on.

Btw. thanks a lot on the help with how to create indexers

@olivernybroe olivernybroe merged commit 143ec8a into master Aug 19, 2020
@olivernybroe olivernybroe deleted the feat-test-finder branch August 19, 2020 13:14
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.

Navigate using "Go to Test" doesn't find pest tests
3 participants