search

sillsdev / languageforge-lexbox

Lexbox, SIL linguistic data hub
MIT License
7 stars 2 forks source link

Validate WritingSystemIds on creation #1230

Closed rmunn closed 1 week ago

rmunn commented 1 week ago

Fix #984.

We now validate WritingSystemId records on creation and throw ArgumentException for any invalid writing system IDs. The special ID value "default" is allowed, all other writing system IDs must be considered valid by the SIL.WritingSystems validation logic.

rmunn commented 1 week ago

Five failing unit tests when I run this locally:

The three FwLiteProjectSync tests are failing because AutoFaker.Generate() is generating randomly-named writing system IDs, and we're no longer allowing that. We'll need to teach Generate() to pick writing system IDs randomly from an allowable list.

... And the two LcmCrdt tests are failing for the same reason: there's an AutoFaker.Generate() call in there, which needs to be taught how to pick valid writing system IDs.

Update: Same five tests are failing in GHA, for the same reason.

rmunn commented 1 week ago

Commit 30bdc0f0 fixes the three FwLiteProjectSync tests. But interestingly, the two failing LcmCrdt tests are still generating writing systems that are randomly-chosen English words, rather than two-letter codes. Something in those tests isn't calling AutoFaker.Generate correctly...

rmunn commented 1 week ago

Commit 5d94a57c passes the MultiStringOverride to the two tests that were failing — but it's not being called correctly. So far I'm not sure why.

rmunn commented 1 week ago

Kevin has asked for some explicit unit tests around writing system ID validation, especially making sure that custom audio writing systems are going to validate. So I'm removing the review request, and I'll bring it back when those tests are written.