Skip to content

Ensure finalization attributes are preserved on LinuxSystemAppConfig#2773

Merged
kattni merged 2 commits intobeeware:mainfrom
freakboy3742:linux-finalization-properties
Mar 28, 2026
Merged

Ensure finalization attributes are preserved on LinuxSystemAppConfig#2773
kattni merged 2 commits intobeeware:mainfrom
freakboy3742:linux-finalization-properties

Conversation

@freakboy3742
Copy link
Copy Markdown
Member

Fixes beeware/toga#4278

When finalising an Linux System app, any finalisation properties that were passed in to the finalize_app_config() method were not being set on the LinuxSystemAppConfig instance. This is because they were being set using the super().finalize_app_config() call, and then overwritten with default values when the LinuxSystemAppConfig instance was constructed.

The manifestation of this - test_mode is never set on Linux system apps - the base implementation of finalize_app_config() sets the attribute; but the LinuxSystemAppConfig narrowing then overwrites the value with the default (test_mode = False).

I looked into a number of possible solutions:

  1. Moving the **kwargs usage to the outer LinuxSystemAppConfig call:
    return LinuxSystemAppConfig(super().finalize_app_config(), **kwargs)
    
    This works, but means that if the base class ever adds anything with the values of kwargs, that functionality wouldn't be triggered
  2. Duplicate the **kwargs over both locations:
    return LinuxSystemAppConfig(super().finalize_app_config(**kwargs), **kwargs)
    
    Again, this works, and any side effects of the base class finalisation would be triggered; but the actual values would be overwritten
  3. Add a mechanism for retrieving the overwritten attributes:
    final_config = super().finalize_app_config(**kwargs)
    return LinuxSystemAppConfig(final_config, **final_config.final_kwargs)
    
    where final_kwargs is returns all the attributes that are passed into the FinalizedAppConfig constructor. This works, but requires a lot of code/attribute duplication
  4. Modify the constructor for FinalizedAppConfig so that the default values for every attribute in the FinalizedAppConfig constructor is an UNDEFINED object rather than actual value, and then only setting the value of the attribute if it is actually provided as an argument.
  5. Modify the constructor of FinalizedAppConfig so that the arguments to the constructor are **kwargs. This way, we can set the values as part of the base class finalisation, but they're not overwritten when the LinuxSystemAppConfig is created.

I would have thought (4) was the "strongest" fix from a typing perspective, but it turns out that (4) and (5) both end up with exactly the same count of typing errors (334), and (5) is a lot simpler to implement, so I've gone with that approach for now.

I've added a test for this specific case - but I've also updated the tests for finalize_app_config to validate that the returned config has all the right attributes (since that is the one that will actually be used). It didn't reveal any new problems in this case, but it easily could do.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Copy link
Copy Markdown
Contributor

@kattni kattni left a comment

Choose a reason for hiding this comment

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

This appears to be a viable fix. Looks good to me.

@kattni kattni merged commit c101596 into beeware:main Mar 28, 2026
60 checks passed
@freakboy3742 freakboy3742 deleted the linux-finalization-properties branch March 28, 2026 03:27
@freakboy3742
Copy link
Copy Markdown
Member Author

@filiplajszczak I'd be interested in your take on this as a fix, since it intersects with the typing work you've been doing. We had to merge it as an emergency fix because it was breaking Toga's CI, and it didn't seem to make ty results any worse.

@filiplajszczak
Copy link
Copy Markdown
Contributor

@freakboy3742 Looks like **kwargs avoids the overwrite problem nicely, and since finalize() is the only call it doesn't lose much type safety in practice.

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.

Linux testbed timing out in CI

3 participants