NightOwl888 commented on code in PR #1154: URL: https://github.com/apache/lucenenet/pull/1154#discussion_r2058205062
########## src/Lucene.Net.Tests.Analysis.SmartCn/DictionaryTests.cs: ########## @@ -0,0 +1,72 @@ +using Lucene.Net.Util; +using Lucene.Net.Analysis.Cn.Smart.Hhmm; +using Lucene.Net.Attributes; +using NUnit.Framework; +using System; +using System.IO; +using System.Reflection; + + +[TestFixture] +[LuceneNetSpecific] +public class DictionaryTests : LuceneTestCase Review Comment: > Would you prefer I keep the inline comments? Yes, the comments provde better context for what the test is doing. Good work. > Do you recommend adding more edge cases or assertion types? The rest of the tests would likely fail if there were any major issues with the dictionary data, so they should cover the rest through integration testing. > Any feedback on the structure or naming before I finalize the PR? The only changes I would suggest: 1. Mark the `CheckBigramDictionary()` and `CheckWordDictionary()` methods `static`, since they don't access any local state. 2. Place the `using Assert = Lucene.Net.TestFramework.Assert;` line after all of the other using directives. Looking forward to reviewing the final draft. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
