Skip to content

Conversation

@subha0319
Copy link

Implements #105.

Updates the TestExamples test suite to check for specific error messages.
The test harness now reads a comment tag // @ExpectedError: "Error Title" from files that are expected to fail and validates the actual error title against the expected one.

subha0319 and others added 2 commits November 8, 2025 23:53
Implements liquid-java#105. The `TestExamples.java` test suite will now read a `// @expectederror: "Error Title"` comment from any
"Error" file and fail the test if the error title does not match.
@CatarinaGamboa
Copy link
Collaborator

Hi @subha0319! Thanks for contributing! Can you add a couple of examples with those annotations to see if it is working? We can also decide to have those annotation lines as the first line in the file, so that it is easier to check it and faster to run the tests

Implements liquid-java#105. The `TestExamples.java` test suite will now read a `// @expectederror: "Error Title"` comment from any
"Error" file and fail the test if the error title does not match.
@subha0319
Copy link
Author

Hi @CatarinaGamboa, Thanks for the feedback.

I've added the ExpectedError annotation to a couple more test files as you suggested: ErrorAfterIf.java and ErrorArithmeticBinaryOperations.java (along with ErrorSimpleAssignment.java which I had updated earlier).

And I've made sure to place the annotations on line 1 in all the examples.

Let me know if there's anything else.

Copy link
Collaborator

@CatarinaGamboa CatarinaGamboa left a comment

Choose a reason for hiding this comment

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

left some comments, we can change the getExpectedError to be more eddicient

@subha0319
Copy link
Author

Hi @CatarinaGamboa,

Thanks for the great review! I've updated the PR with all your suggestions:

  • I've changed the logic to use a .contains() check, so the error tags can be more general and robust (e.g., // @ExpectedError: "Type expected"). I've updated the example files to use these general tags.

  • I've refactored the getExpectedError method to return an Optional instead of null, which is much cleaner.

  • Since we agreed the tag should be on the first line, I've also changed the file reader to only check the first line using findFirst().

Let me know if there's anything else!

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