comtypes icon indicating copy to clipboard operation
comtypes copied to clipboard

Tests are broken?

Open dmwyatt opened this issue 2 years ago • 7 comments

I'm trying to run the tests, but I can't. Is anyone able to run the tests successfully? I'm on Windows 10 using python 3.10 64 bit.

Here's some of the issues I'm having. They might belong in separate issues here or they might all be because the tests just don't run on python 3. Hopefully we can figure that out.

  1. Getting multiple NameError: name 'basestring' is not defined errors from the following line. I guess this is the same issue we talk about with long integers in PR #259.
  2. test_avmc.py fails like so:
    Error
    Traceback (most recent call last):
      File "C:\Users\dustin\PycharmProjects\comtypes\comtypes\test\test_avmc.py", line 9, in test
        avmc = CreateObject("AvmcIfc.Avmc.1")
      File "C:\Users\dustin\PycharmProjects\comtypes\comtypes\client\__init__.py", line 227, in CreateObject
        clsid = comtypes.GUID.from_progid(progid)
      File "C:\Users\dustin\PycharmProjects\comtypes\comtypes\GUID.py", line 85, in from_progid
        _CLSIDFromProgID(text_type(progid), byref(inst))
      File "_ctypes/callproc.c", line 993, in GetResult
    OSError: [WinError -2147221005] Invalid class string
    
  3. The tests test_leaks_1, test_leaks_2, test_leaks_3 in test_collections.py take almost 7 minutes to complete on my very beefy machine and test_leaks_2 fails with AssertionError: 688128 is not false : Leaks 688128 bytes.
  4. Some tests fail with:
    comtypes.test.ResourceDenied: Use of the `events' resource not enabled
    
  5. Some tests fail with:
    comtypes.test.ResourceDenied: Use of the `ui' resource not enabled
    

There's a lot more failures but I'll get to posting them if we can make any progress on fixing these.

dmwyatt avatar Feb 02 '22 20:02 dmwyatt

Relatedly, if we can get tests running I can help set up some automatic testing on PRs so the tests are less likely to get into this state again.

dmwyatt avatar Feb 02 '22 21:02 dmwyatt

I think we can add automatic tests to appveyor.yaml in addition to current installation tests only. I had no time to look into test failures, but I remember some failures are there. It would be really nice to revive the whole suite or at least the most part of it.

vasily-v-ryabov avatar Feb 03 '22 12:02 vasily-v-ryabov

So, I'm devoting some time to figuring out whats going on with the tests.

There are multiple tests that depend on Internet Explorer. The problems with this:

  1. It requires the user to have manually run Internet Explorer in the past and accepted an option on the first start up of IE that requires you to choose default security settings.
  2. I've tested this on three different systems and you just can't access the Document property like the MS documentation seems to indicate you should be able to. You just get an AttributeError from comtypes.
  3. The tests load pages from the internet into IE. This is very fragile!
  4. Windows can have IE turned off so it's not even available.

I think we should just drop the whole test/test_ie.py file, and rewrite the other tests that just incidentally use IE to use something else.

In general the tests are pretty poor quality by modern testing standards and mostly all need rewritten, but I think I can get a decent subset of them going without too much work. Unless someone objects I'm going to come up with an initial PR that contains a working test suite. It will drop maybe 25%-50% of the tests that don't work for various reasons and then we can have a discussion on the PR about if I made the right decisions.

It seems like it'd be better to have a test suite that runs but doesn't cover everything than a test suite that doesn't run but hypothetically covers more.

Any disagreements?

dmwyatt avatar Feb 03 '22 19:02 dmwyatt

The tests should me a mixture of querying known Windows API COM interfaces that have a known output and also be a self contained there where a COM interface gets set up with different test functions and a connection to that interface would be made passing data between them.

Because of the scope of what comtypes does every single facet of it cannot be tested.

kdschlosser avatar Feb 05 '22 03:02 kdschlosser

@dmwyatt I totally agree. From my side I can do external testing in pywinauto repository that depends on comtypes. Fortunately we can use direct link to zipped branch of comtypes in pywinauto's requirements.txt for test purpose. So I can test pywinauto is not broken before comtypes' release.

True unit tests must not rely on external services as much as possible. Maybe it's worth to mock some external things, though it may take time. Anyway guys I appreciate your efforts and will try to review all your PRs in a one week timeframe.

vasily-v-ryabov avatar Feb 05 '22 16:02 vasily-v-ryabov

I created the preliminary PR #271.

dmwyatt avatar Feb 06 '22 22:02 dmwyatt

Just a reminder that I noticed when looking at #96.

The __verbose__ attribute has been patched which is not used in the production code.

https://github.com/enthought/comtypes/blob/7e90e54d2131b35edef3e06f98f26a255f3dfa42/comtypes/test/test_createwrappers.py#L26-L27

The entire test_createwrappers module is skipped, so we should do something about that first, but my priority has shifted to another part, so I'll just write it down.

junkmd avatar Dec 06 '22 08:12 junkmd