Skip to content

Conversation

johnzhou721
Copy link
Contributor

@johnzhou721 johnzhou721 commented Jun 4, 2025

Refs #3529.

Enable lots of testing things as done by @danyeaw in #3239 sans TextInput things. Most tests are passing on Ubuntu 22.04.

Revert a352470 and 1e03f9a in that reversion order to re-enable textinput.

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

danyeaw and others added 30 commits May 20, 2025 15:49
Co-authored-by: Muhammad Murad <[email protected]>
Co-authored-by: Muhammad Murad <[email protected]>
Co-authored-by: Muhammad Murad <[email protected]>
This commit resolves variable binding issues in the test_cleanup function
when used with pytest's parameterization. The errors appear as runtime
NameErrors.
@danyeaw
Copy link
Member

danyeaw commented Jun 5, 2025

Hi @johnzhou721 , it looks like there is some work to do here to skip the tests for TextInput and make sure the tests are passing for this widget.

@johnzhou721

This comment was marked as resolved.

@johnzhou721

This comment was marked as resolved.

@johnzhou721
Copy link
Contributor Author

Should be fixed.

@johnzhou721
Copy link
Contributor Author

Interesting. There's a window failure on 22.04 but not on CI, and there's a fixed square widget size failure on CI (always) but never on my end. We'd prefer to resolve both before merging this PR, but I am completely stuck.

@johnzhou721
Copy link
Contributor Author

@danyeaw Do you have any idea why the size is reporting 0? It seems like I'm doing all the right things and it passes with an older version of GTK4. Could it be related to the queue_draw issue you're looking at?

Thanks

@danyeaw
Copy link
Member

danyeaw commented Jun 8, 2025

Hi @johnzhou721, if this is only failing on CI and not locally, then it is probably the same issue.

@johnzhou721
Copy link
Contributor Author

@danyeaw How would we go about resolving this then? Where should I go looking in the gtk sources? Because this is a huge blocker for any implementations of any GTK widget.

@danyeaw
Copy link
Member

danyeaw commented Jun 9, 2025

Hi @johnzhou721 , I'm trying to research this in the GTK source code. If you want to see what you can find that is great or you can hold off and work on other items for the time being

@johnzhou721
Copy link
Contributor Author

Hi @danyeaw, is the widget "mapped" in CI? If not it gets filtered out. But the same thing exist in GTK3 sources just hidden deeperly, so not sure if this is the cause.

If this is not it, we need to start looking into GDK. GTK seems to be doing the right thing.

@danyeaw
Copy link
Member

danyeaw commented Jun 11, 2025

Hi @johnzhou721 , what do you mean by mapped?

@johnzhou721
Copy link
Contributor Author

@johnzhou721
Copy link
Contributor Author

@danyeaw Not sure if the internal mapped flag is related to https://docs.gtk.org/gtk4/signal.Widget.map.html but that latter says "A widget is mapped when the widget is visible [...] and all its parents up to the toplevel widget are also visible."

@danyeaw
Copy link
Member

danyeaw commented Jun 11, 2025

Yes, I understand that to GTK, the widgets are mapped and visible. GTK doesn't know that it is drawing to a virtual framebuffer on CI.

@johnzhou721
Copy link
Contributor Author

@danyeaw OK... sorry for not knowing some of the perks of CI... closing the loop on this for now.

@johnzhou721
Copy link
Contributor Author

@danyeaw Any progress here, or are you still investigating?

Also, is there a way for us to monitor all calls to GTK native APIs while the test suite is running? If so, we could log all the calls and use it to write a C reproducer to report upstream.

I don't think this is possible because while you can intercept getattr for getting the function property, of e.g. Gio, you don't know what arguments the function is being called with... does there exist a feature in Python when you can intercept method calls on a class to grab the method name and its arguments, so we can pass it on to GTK as well as print the call?

@johnzhou721
Copy link
Contributor Author

Wait... you can define a wrapper function that grabs args and kwargs and forward them to the native GTK function!

Untested code (do Proxy(Gio) and assign it to the Gio the module exports or something like that...) I unfortunately having a test coming up in a few days, don't have enough time to implement or test this... so maybe danyeaw can help? Thanks!

class Proxy:
    def __init__(self, wrapped):
        self._wrapped = wrapped

    def __getattr__(self, name):
        attr = getattr(self._wrapped, name)
        if callable(attr):
            def wrapper(*args, **kwargs):
                print(f"Calling method: {name} args={args} kwargs={kwargs}")
                result = attr(*args, **kwargs)
                return result
            return wrapper
        else:
            print("Accessing", name)
            return attr

@danyeaw
Copy link
Member

danyeaw commented Jul 10, 2025

Hey @johnzhou721 - unfortunately I haven't had a much time to dig in to this as I would like. I think we really need to root cause this upstream though since BeeWare depends on the full stack of libraries working together, not just the BeeWare code. Hopefully I can get back to working on this more soon

@johnzhou721
Copy link
Contributor Author

@danyeaw Totally fine! Will see if I can execute my (sort of tough) plan about capturing all system calls, but it might be 6--8 weeks later b/c the rest of my summer is packed...

@johnzhou721
Copy link
Contributor Author

Also, fair winds and following seas if you're investigating this further.

@johnzhou721
Copy link
Contributor Author

I am unable to investigate this issue any further because my VM is basically not working at all. I'd need to clean up my disk before I can get a new one up and running (because I archived my old VM disk... and it still takes disk space), and the amount of time I'm spending with togax_qt means I can't work on this at the same time as well.

Since I'm not going to work on this until perhaps a few months from now, I will close this PR (and also since it duplicates a lot of the TextInput PR); changes required to the widget can be found in a9570a4, and the skip in the probe should be removed. I explicitly give permission for anyone to reuse this commit for any other PRs on this repo.

@danyeaw TL;DR Use the TextInput PR if you have any further progress, and feel free to add my ActivityIndicator code after that is up and running.

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