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

Tests are broken? #267

Open
dmwyatt opened this issue Feb 2, 2022 · 7 comments
Open

Tests are broken? #267

dmwyatt opened this issue Feb 2, 2022 · 7 comments
Labels
enhancement New feature or request tests enhance or fix tests

Comments

@dmwyatt
Copy link
Contributor

dmwyatt commented Feb 2, 2022

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 Restore Python2.7 support without 2to3 fixer + timestamping fix #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
Copy link
Contributor Author

dmwyatt commented Feb 2, 2022

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.

@vasily-v-ryabov
Copy link
Collaborator

vasily-v-ryabov commented Feb 3, 2022

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 vasily-v-ryabov added the enhancement New feature or request label Feb 3, 2022
@dmwyatt
Copy link
Contributor Author

dmwyatt commented Feb 3, 2022

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?

@kdschlosser
Copy link
Contributor

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.

@vasily-v-ryabov
Copy link
Collaborator

@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.

@dmwyatt dmwyatt mentioned this issue Feb 6, 2022
3 tasks
@dmwyatt
Copy link
Contributor Author

dmwyatt commented Feb 6, 2022

I created the preliminary PR #271.

@junkmd
Copy link
Collaborator

junkmd commented Dec 6, 2022

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

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

# don't print messages when typelib wrappers are generated
comtypes.client._generate.__verbose__ = False

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tests enhance or fix tests
Projects
None yet
Development

No branches or pull requests

4 participants