eclipse-xtext / eclipse-xtext/xtext
Deal with end-of-lines in AbstractQuickfixTest independently from OS
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 831
- Forks
- 330
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 12
Description
The AbstractQuickfixTest does not allow to easily customize comparison of strings in the presence of Windows end-of-line characters like "\r". That's especially the case when using new Java text blocks (which never contain \r) or in some cases where the quickfix might leave some spurious \r after the applications. Note that using the trick (used in Xtext tests itself):
@Override
public void testQuickfixesOn(CharSequence content, String issueCode, Quickfix... quickfixes) {
super.testQuickfixesOn(Strings.toUnixLineSeparator(content), issueCode, quickfixes);
}
does not help because of the above-mentioned spurious \r and Java text blocks.
A "drastic" way to solve the problem would be to always remove \r in org.eclipse.xtext.ui.testing.AbstractQuickfixTest.assertIssueResolutionResult(String, IssueResolution, String), e.g., something like
protected void assertIssueResolutionResult(String expectedResult, IssueResolution actualIssueResolution, String originalText) {
/*
* Manually create an IModificationContext with an XtextDocument and call the
* apply method of the actualIssueResolution with that IModificationContext
*/
IXtextDocument document = getDocument(originalText);
TestModificationContext modificationContext = new TestModificationContext();
modificationContext.setDocument(document);
new IssueResolution(actualIssueResolution.getLabel(), //
actualIssueResolution.getDescription(), //
actualIssueResolution.getImage(), //
modificationContext, //
actualIssueResolution.getModification(), //
actualIssueResolution.getRelevance()).apply();
String actualResult = document.get();
assertEquals(
Strings.toUnixLineSeparator(expectedResult),
Strings.toUnixLineSeparator(actualResult));
}
After all, I don't see why one would want to deal with \r (from the text document of the editor point of view, nothing changes in that respect).
Note that currently you can't fix the problem by simply overriding assertIssueResolutionResult: you could override that method, but "TestModificationContext" is private so there's not much one can do; I personally deal with this problem by using a custom XtextDocument in the tests, but I'd like to provide a cleaner solution.
If always using \n instead of \r\n as suggested above scares because of possible breakage in existing tests, at least, I would allow developers to redefine the comparison part, e.g., assertIssueResolutionResult could delegate the equal assertion to a protected method that the developer can redefine.
I can provide a PR once we somehow agree on a possible solution.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in org.eclipse.xtext.ui.testing.AbstractQuickfixTest, especially assertIssueResolutionResult(String, IssueResolution, String), and inspect how TestModificationContext applies resolutions. Determine whether line-ending normalization or a protected comparison hook best fits the existing API; done means quickfix tests can compare results reliably across Windows line endings and Java text blocks without breaking existing tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- testing
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100