Skip to content

Fix LT-21889: Add Find and Fix step for removing disowned children#372

Merged
jtmaxwell3 merged 2 commits intomasterfrom
LT-21889
May 4, 2026
Merged

Fix LT-21889: Add Find and Fix step for removing disowned children#372
jtmaxwell3 merged 2 commits intomasterfrom
LT-21889

Conversation

@jtmaxwell3
Copy link
Copy Markdown
Contributor

@jtmaxwell3 jtmaxwell3 commented May 1, 2026

This fixes https://jira.sil.org/browse/LT-21889. If the owner and stored owner don't match and the owner exists and the stored owner doesn't exist then the node has been disowned. This revealed some problems with the unit tests (type="o" instead of t="o").


This change is Reviewable

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

LCM Tests

    16 files  ±0      16 suites  ±0   3m 12s ⏱️ +12s
 2 858 tests ±0   2 838 ✅ ±0   20 💤 ±0  0 ❌ ±0 
11 380 runs  ±0  11 212 ✅ ±0  168 💤 ±0  0 ❌ ±0 

Results for commit bc3b0a9. ± Comparison against base commit c459605.

♻️ This comment has been updated with latest results.

@jasonleenaylor
Copy link
Copy Markdown
Contributor

tests/SIL.LCModel.FixData.Tests/FwDataFixerTests.cs line 484 at r1 (raw file):

			Assert.That(_errors[8], Is.EqualTo("Removing dangling link to Form '" + danglingMsaGuid + "' for WfiMorphBundle '" + danglingMorphNoRepairGuid + "'."),
				"Error message is incorrect."); // MorphBundleFixer--ksRemovingDanglingMorph
			Assert.True(_errors[9].StartsWith("Removing disowned object (invalid ownerguid='" + disownedOwnerGuid),

The added assert for the error message is good, but we should also have a new assert for the actual data being removed. I expected to see an assert below in the // Check file repair section

Copy link
Copy Markdown
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

@jasonleenaylor partially reviewed 6 files and made 1 comment.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on jtmaxwell3).

@jtmaxwell3 jtmaxwell3 merged commit b87d9f9 into master May 4, 2026
4 of 5 checks passed
@jtmaxwell3 jtmaxwell3 deleted the LT-21889 branch May 4, 2026 17:12
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