Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(web): proper disabling of prediction timeout for prediction unit tests #9835

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Oct 24, 2023

While I'd previously added a "test mode" flag for use in prediction-oriented unit tests... it appears I somehow completely forgot to actually use it in the unit test specs. This remedies that little issue.

From the ModelCompositor definition in lm-worker:

private testMode: boolean = false
constructor(lexicalModel: LexicalModel, testMode?: boolean) {
this.lexicalModel = lexicalModel;
if(lexicalModel.traverseFromRoot) {
this.contextTracker = new correction.ContextTracker();
}
this.punctuation = ModelCompositor.determinePunctuationFromModel(lexicalModel);
this.testMode = !!testMode;
}

const SEARCH_TIMEOUT = this.testMode ? 0 : correction.SearchSpace.DEFAULT_ALLOTTED_CORRECTION_TIME_INTERVAL;
for(let matches of searchSpace.getBestMatches(SEARCH_TIMEOUT)) {

But... so many unit tests forgot to actually specify the parameter. 😦

@keymanapp-test-bot skip

@jahorton jahorton requested a review from mcdurdin as a code owner October 24, 2023 05:27
@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S24 milestone Oct 24, 2023
@jahorton jahorton changed the title fix(web): someone forgot to actually use 'test mode' fix(web): proper disabling of prediction timeout for prediction unit tests Oct 24, 2023
@jahorton jahorton merged commit a75cd6d into master Oct 24, 2023
3 checks passed
@jahorton jahorton deleted the fix/common/models/prediction-test-mode-use branch October 24, 2023 09:33
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.197-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants