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

Another view on how tests should like #41

Merged
merged 3 commits into from
Jan 26, 2023
Merged

Another view on how tests should like #41

merged 3 commits into from
Jan 26, 2023

Conversation

max-martynov
Copy link
Collaborator

When I started, I was thinking that this new tests system will be more concise and handy. Maybe it's just the problem in PHP bindings from ControlFlowInstructions to PsiElements but I find it still hard to write tests like that. However it can still be better than the previous solution.

Anyway, for referencing PsiElements I decided to go with the following triple: some text representation of a PsiElement, line number in the source file and column number in the source file where the PsiElement is located.

As a text representation I decided to use the simple PsiElement.toString() method because it's mostly the same content as the PsiViewer shows us. Another advantage is that keeps text short even for structures like for or if.

I tried not to add the column number but keep struggling with the corner cases like that:

 for ($i = 0; $i < 2; $i++)
 {
...
}

Here $i in $i = 0 and $i in $i < 2 correspond to 2 different PsiElements (they are both VariableImpls but still). So that's why I find column number necessary.

@max-martynov max-martynov changed the base branch from php-control-flow to main January 26, 2023 19:33
@egor-bogomolov egor-bogomolov merged commit bee0af8 into main Jan 26, 2023
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.

2 participants