Fix bug with realizational rules

[jtmaxwell3]

ExpandAlternatives didn't work correctly with realizational rules. I didn't notice this before because ExpandAlternatives treated originals.Count == 1 as a special case, and bypassed the code for reconstructing the alternative starting at line 423. Disabling the special case code caused the RealizationRule unit test to fail. All of the unit tests work now, but if you can think of anything else the code for reconstructing the alternative should include, please let me know. Ideally, the code for reconstruction would figure out what changes were made between Source and this, and then apply those changes to the alternative.

---

This change is 

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 12.50000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.28%. Comparing base (9433640) to head (93f3336).
Report is 39 commits behind head on master.

Files with missing lines	Patch %	Lines
src/SIL.Machine.Morphology.HermitCrab/Word.cs	12.50%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
- Coverage   70.29%   70.28%   -0.02%     
==========================================
  Files         385      385              
  Lines       32012    32019       +7     
  Branches     4503     4504       +1     
==========================================
  Hits        22503    22503              
- Misses       8465     8471       +6     
- Partials     1044     1045       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ddaspit]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jtmaxwell3)

[ddaspit]

Did the unit tests not catch this issue? Maybe we should add some unit tests.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jtmaxwell3)

[jtmaxwell3]

The unit tests would have caught the issue, except that I had a performance optimization that bypassed the problematic code unless there were alternatives. One solution would be to turn off the performance optimization when testing via a static Word.Testing property. Otherwise, we have to have make a new version of every unit test that includes alternatives to fully test ExpandAlternatives.

[ddaspit]

Can we add a unit test that does have alternatives just to test that code path?

Reviewable status: complete! all files reviewed, all discussions resolved

[jtmaxwell3]

We have a unit test with alternatives, so that code path is tested. It is just that we didn't have a unit test with alternatives AND a realization rule. To test all possible combinations of alternatives and other phenomena, we would need to double the number of unit tests. Or we could turn off the optimization during testing so that ExpandAlternatives get exercised even if there are no alternatives.

[ddaspit]

At the very least, could we add a unit test for the specific case of a realizational rule with alternatives?

[jtmaxwell3]

It is a pain to create alternatives, since you have to come up with an alternative set of rules that are different from the existing rules but produce the same results. Turning off the optimization during testing is more general, and works for future rule types as well. But if you don’t want to turn off the optimization during testing, I will create a specific case of a realization rule with alternatives. On Jan 24, 2025, at 8:01 AM, Damien Daspit ***@***.***> wrote: At the very least, could we add a unit test for the specific case of a realizational rule with alternatives? — Reply to this email directly, view it on GitHub <#285 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVKIQNWHOPCSBU3XKGIBAT2MJPUJAVCNFSM6AAAAABVXWYFQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJSHA3DSMJQGE>. You are receiving this because you were mentioned.

[ddaspit]

I would prefer not to turn off the optimization for tests, because I want the tests to run on the shipped code.

Whenever we fix a bug, I would like a unit test to verify that the bug was fixed and to protect against regressions. It would be very helpful if you could add a unit test for the specific case.