LT-21638: Stop SFM import crashing on a blank Database File by thejambi · Pull Request #1231 · sillsdev/FieldWorks

thejambi · 2026-11-05T03:05:34Z

Clicking Save in the Standard Format lexicon import wizard, or answering Yes to "Save settings?" after Cancel, no longer crashes when the Database File box is blank or names a file that does not exist. The wizard says what is wrong and stays on the page: "The database file must be defined." for a blank box, or "The selected database file (...) does not exist." for a missing one. Finish and Quick Finish give the same warnings, and Cancel closes without offering to save.

Next on the first page also stays disabled until the database file exists. Until now a missing file whose name ended in PhaseNOutput.xml let Next jump straight to the import step, which then failed.

Reviewer notes

Root cause. Saving settings first runs every wizard step to fill in its data, and the marker step calls File.GetLastWriteTime on the Database File path, which throws "The path is not of a legal form" for a blank path. The 2023 fix (43507dc) skipped the save prompt on Cancel only when the box was completely empty, so QA's later repro on the ticket (choose a file, clear the box, click Save) went around it.

Deliberate rules.

UsesInvalidFileNames rejects a blank or missing database file before checking anything else. It already runs before Save, Finish, Quick Finish, Next (step 2) and Cancel, so one guard covers every way into the step rebuild, and the separate empty-string test on Cancel is folded into it. Returning early also skips SetDefaultSettings, which derives the save-as name from the database path.
In OnNextButton the phase-file jump to the import step is an else if after that check, so a warning is never followed by navigation.
CanLeaveFileStep decides when Next is enabled on the first page: the database file must exist, phase output files included. Other files are judged as before, including the rule against saving over the shipped MDFImport.map. It is static so tests can call it without a form.
Two new strings, ksUndefinedDatabaseFile and ksInvalidDatabaseFileX, follow the wording of the existing settings-file messages. No existing resource key changed meaning.

Tests. New LexImportWizardTests cover CanLeaveFileStep with missing and existing phase files, blank and whitespace-only paths, saving over the shipped map, and a valid file; none of them constructs the wizard form. The missing-phase-file case fails against the old code. UsesInvalidFileNames and the Save, Cancel and Next handlers are not unit-tested, because exercising them means building the form, and the handlers open modal message boxes. LexTextControlsTests: 413 passed, 3 skipped; the build and comment and token hygiene are clean.

Manual tests (not done). The acceptance steps on LT-21638 have not been run in FLEx; QA covers them on the nightly build.

Out of scope. The wizard has no Avalonia counterpart, so there is nothing to keep in step.

🤖 Generated with Claude Code

This change is

In File > Import > Standard Format Lexicon, clicking Save, or answering Yes to the Save settings? question on Cancel, crashed with "The path is not of a legal form" when the Database File box was blank or named a file that does not exist. Saving fills in every wizard step first, and the marker step reads the input file's timestamp. The wizard's file-name check now rejects a blank or missing Database File before anything else. Save, Finish and Quick Finish warn and stay put; Cancel closes without offering to save, as it already did for an empty box. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-11-05T03:22:07Z

NUnit Tests

1 files ± 0 1 suites ±0 11m 24s ⏱️ - 4m 6s
7 057 tests +16 6 972 ✅ +16 85 💤 ±0 0 ❌ ±0
7 066 runs +16 6 981 ✅ +16 85 💤 ±0 0 ❌ ±0

Results for commit 1be110b. ± Comparison against base commit e455002.

♻️ This comment has been updated with latest results.

codecov-commenter · 2026-11-05T03:22:11Z

Codecov Report

❌ Patch coverage is 15.78947% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.51%. Comparing base (e455002) to head (1be110b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/LexText/LexTextControls/LexImportWizard.cs	15.78%	11 Missing and 5 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1231   +/-   ##
=======================================
  Coverage   40.51%   40.51%           
=======================================
  Files        1559     1559           
  Lines      356080   356091   +11     
  Branches    41585    41589    +4     
=======================================
+ Hits       144262   144288   +26     
+ Misses     182505   182491   -14     
+ Partials    29313    29312    -1

Files with missing lines	Coverage Δ
Src/LexText/LexTextControls/LexImportWizard.cs	`0.59% <15.78%> (+0.10%)`	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

A Database File named PhaseNOutput.xml that does not exist showed the new missing-file warning, and then Next still jumped to the import step. The jump to the import step now runs only when the file check passes, and Next stays disabled on the file step until the database file exists, phase output files included. The UsesInvalidFileNames summary now states only its own contract. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jasonleenaylor

Clean fix. One question on the tests.

jasonleenaylor · 2026-11-05T17:34:14Z

+		public void UsesInvalidFileNames_BlankDatabaseFile_IsInvalid(string databaseFile)
+		{
+			var settingsFile = CreateFile("import-settings.map");
+			using (var wizard = new LexImportWizard())


We'd like to avoid constructing WinForms forms in unit tests. new LexImportWizard() runs the designer's InitializeComponent(), and these tests depend on its TextChanged handlers staying inert before the form loads. Could these three tests either test a no-WinForms helper, like you did with CanLeaveFileStep, or be dropped? The database-file check is a single File.Exists, so they may not be testing anything of value.

Thanks! Those tests have been removed.

The UsesInvalidFileNames tests constructed LexImportWizard and so ran the designer code, relying on its text-changed handlers doing nothing before the form loads. The rule they checked, that a blank or missing database file is unusable, is covered by the CanLeaveFileStep tests, which need no form. UsesInvalidFileNames is private again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

thejambi · 2026-11-05T19:50:47Z

+		public void UsesInvalidFileNames_BlankDatabaseFile_IsInvalid(string databaseFile)
+		{
+			var settingsFile = CreateFile("import-settings.map");
+			using (var wizard = new LexImportWizard())


Thanks! Those tests have been removed.

thejambi marked this pull request as ready for review November 5, 2026 16:46

jasonleenaylor approved these changes Nov 5, 2026

View reviewed changes

thejambi commented Nov 5, 2026

View reviewed changes

thejambi merged commit 412ee26 into main Nov 5, 2026
8 of 9 checks passed

thejambi deleted the LT-21638-sfm-import-empty-path-crash branch November 5, 2026 20:08

Navigation Menu

Repository navigation

LT-21638: Stop SFM import crashing on a blank Database File - #1231

LT-21638: Stop SFM import crashing on a blank Database File#1231
thejambi merged 3 commits into
mainfrom
LT-21638-sfm-import-empty-path-crash

thejambi commented Nov 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 5, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Nov 5, 2026 •

edited

Loading

Uh oh!

jasonleenaylor left a comment

Uh oh!

jasonleenaylor Nov 5, 2026

Uh oh!

thejambi Nov 5, 2026

Uh oh!

thejambi Nov 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

thejambi commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

NUnit Tests

Uh oh!

codecov-commenter commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jasonleenaylor left a comment

Choose a reason for hiding this comment

Uh oh!

jasonleenaylor Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

thejambi Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

thejambi Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thejambi commented Nov 5, 2026 •

edited

Loading

github-actions Bot commented Nov 5, 2026 •

edited

Loading

codecov-commenter commented Nov 5, 2026 •

edited

Loading