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

Failing test: TestBufferedCharFilter.Test_Ready #1102

Open
1 task done
paulirwin opened this issue Jan 16, 2025 · 1 comment
Open
1 task done

Failing test: TestBufferedCharFilter.Test_Ready #1102

paulirwin opened this issue Jan 16, 2025 · 1 comment
Labels
good-first-issue Good for newcomers is:bug test-failure up-for-grabs This issue is open to be worked on by anyone

Comments

@paulirwin
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

The TestBufferedCharFilter.Test_Ready test was missing the [Test] attribute, which was added in #1101. This test failed, however. It fails because it requires the provided TextReader to be a CharFilter where IsReady is true, and the test provides a StringReader that is not a CharFilter. We would need to determine if the upstream Harmony test was ported incorrectly, or if we need to find or create a CharFilter implementation that can be "ready" for this test to pass.

Expected Behavior

The test passes.

Steps To Reproduce

No response

Exceptions (if any)

No response

Lucene.NET Version

No response

.NET Version

No response

Operating System

No response

Anything else?

No response

@paulirwin paulirwin added good-first-issue Good for newcomers is:bug test-failure up-for-grabs This issue is open to be worked on by anyone labels Jan 16, 2025
@paulirwin paulirwin added this to the 4.8.0-beta00018 milestone Jan 16, 2025
@NightOwl888
Copy link
Contributor

A little insight into this. The Reader class in Java tracks some extra state that the .NET TextReader class does not. But this extra state is only required for a small number of classes in Lucene. So, rather than subclassing TextReader with a Reader class with the extra state and having to use adapters for every type of TextReader from .NET to turn it into a Reader, a new type named BufferedCharFilter was added to include both the functionality of CharFilter and the state from BufferedReader (which subclasses Reader) for those few types that required this extra tracking. This puts them in a different arrangement than in Java, so we need to account for this rewiring in the busines logic of BufferedCharFilter. The Harmony tests are from the BufferedReader class to ensure BufferedCharFilter does everything that BufferedReader does. Since this test was missing the [Test] attribute, it isn't too surprising that it doesn't pass by default and we need to analyze how the initial state of IsReady is managed in Java to fix the problem.

Specifically, the extra state tracked in Reader is:

  • mark (an int to track a position)
  • ready (a bool to indicate whether the stream is ready to be read)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Good for newcomers is:bug test-failure up-for-grabs This issue is open to be worked on by anyone
Projects
None yet
Development

No branches or pull requests

2 participants