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

Adding test cases for folding #1960

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

jakub-suliga
Copy link
Contributor

I have added test cases to validate the current folding mechanism.

@jakub-suliga jakub-suliga force-pushed the feature/foldingtest branch 3 times, most recently from e041207 to 69cc34b Compare January 22, 2025 09:28
@jakub-suliga
Copy link
Contributor Author

jakub-suliga commented Jan 22, 2025

@fedejeanne @iloveeclipse Could you review this PR?

@danthe1st
Copy link

danthe1st commented Jan 22, 2025

I assume you will need different tests for your folding of control structures PR (#1562).
Given that (and that I will also need to add test cases related to folding in my PR adding custom folding blocks via comments (#1825), I would suggest creating one test suite for folding tests that delegates to the different test classes for different purposes (for now just your FoldingTest).

For example, this could be

@Suite
@SelectClasses({
	FoldingTest.class,
	// to be added in the future - not for your PR, I am just including that for your understanding
	//ControlStructureFoldingTest.class,
	//CustomFoldingRegionTest.class
})
public class FoldingTestSuite {
}

It should then be possible to reference that test suite in JdtTextTestSuite.

Note: I am not experienced with JDT, I am just making a suggestion.

Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

Looks pretty good, I think it covers most of the existing cases today. I would add some more test cases though.

@jakub-suliga
Copy link
Contributor Author

I assume you will need different tests for your folding of control structures PR (#1562). Given that (and that I will also need to add test cases related to folding in my PR adding custom folding blocks via comments (#1825), I would suggest creating one test suite for folding tests that delegates to the different test classes for different purposes (for now just your FoldingTest).

@fedejeanne @iloveeclipse What do you think of this? I’m capable of implementing it, but I don’t have enough experience to decide if this is the best way to structure the tests.

@jakub-suliga jakub-suliga force-pushed the feature/foldingtest branch 3 times, most recently from e904cae to fe1cd9f Compare January 24, 2025 06:57
@jakub-suliga jakub-suliga force-pushed the feature/foldingtest branch 3 times, most recently from f1c39ba to 621eaef Compare January 24, 2025 08:48
@jakub-suliga
Copy link
Contributor Author

@jjohnstn Could you please review this code too? I have added the checks for the region values as you suggested in #1562. I will also add these checks in the other PR later. Additionally, could you let me know if the above proposal looks good?

@jjohnstn
Copy link
Contributor

@jjohnstn Could you please review this code too? I have added the checks for the region values as you suggested in #1562. I will also add these checks in the other PR later. Additionally, could you let me know if the above proposal looks good?

Looks good. I will merge if the CodeQL check goes through or doesn't finish in the next few minutes.

@jjohnstn jjohnstn merged commit b9e7119 into eclipse-jdt:master Jan 24, 2025
9 checks passed
@jakub-suliga jakub-suliga deleted the feature/foldingtest branch January 29, 2025 04:29
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.

4 participants