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

Make test suite runnable #271

Closed
wants to merge 36 commits into from
Closed

Make test suite runnable #271

wants to merge 36 commits into from

Conversation

dmwyatt
Copy link
Contributor

@dmwyatt dmwyatt commented Feb 6, 2022

Not ready to be merged as-is if python2.7 support is required. Test suite runs on python3 but I still need to get it working on python2.7. It'd be good to get a sanity check from some other people. You can run the tests as described in comtypes/test/README.md

(can we drop 2.7 yet?)

So, I basically just skipped a bunch of tests so that the test suite will run. (oh man that makes it sound so easy, but it wasn't!) A total of 28 out of 110 tests are skipped. Here's what I get on two Windows 10 machines:

❯ python -m unittest discover
......ss..............ss.sss....ss.s.......s....sssss.s....s..s...........ss...ss....s.....ss..............s.s
----------------------------------------------------------------------
Ran 110 tests in 0.601s

OK (skipped=28)

Prior to this PR the test suite wouldn't even run.

(The "28 out of 110 tests" bit is kind of wrong because test_createwrappers.py programmatically creates thousands of tests. I skip this test module because 1) a few dozen of the tests it creates fails and 2) it's dependent upon IE and MS Office)

I imagine an iterative process where we start with the minimum of this initial PR and then build a better test suite going forward.

Instead of outright deleting tests, I used unittest's ability to mark tests for skipping. This way we can maybe evolve the skipped tests to something usable in the future. However, it wouldn't hurt my feelings to just flat-out delete them and start from scratch. In that case we need to come up with a list of functionality in those skipped tests that we should write new tests for.

There's five categories of skipped tests:

  1. It's an explicit test of Internet Explorer, Excel, or Word.
  2. It's a test of something else that happens to use Internet Explorer, Excel, or Word.
  3. It's a test that is failing for reasons I do not understand because it wasn't immediately obvious what the problem was.
  4. It's a test that depends upon comtypes/test/TestComServer.tlb or comtypes/test/TestDispServer.tlb, but these don't work for some reason.
  5. It's a test that causes the whole test suite to fail for some reason.

This PR also includes fixes to all of the minor issues I came across that were causing different tests to fail or even fail to run.

So, tentative short-term next steps for me:

  • Gather sanity checks from some other people's systems (running python 3 for now).
  • Get test suite running on 2.7 😭
  • get tox configured to run tests on a variety of python versions. I'm not familiar with the current CI system for this package, but it'd be neat if, in addition to multiple python versions we could get it running on multiple Windows versions.

Longer-term:

  • Evaluate all the skipped tests and see if they should be:
    • deleted
    • rewritten
    • bug-fixed
  • Evaluate which parts of comtypes are not tested thoroughly.
  • Cleanup the active tests. For example, there's lots of tests that are just called test instead of test_generates_in_memory_module or whatever.
  • Create alternative test suites that can run comtypes against environment-specific stuff. For example, while tests against MS Office aren't appropriate for unit tests we may want to be able to run an Office integration test suite while developing.
  • Personally, I'd rather use pytest. It'd be nice to have dev dependencies so we could use that.

Related issue: #267

This module causes the whole test suite to fail.  Why?  I don't know!
These tests take many minutes to run on my machines. This is too long for unit tests!

They either need discarded or moved to some sort of integration testing suite that can be run outside of the unit test suite.
I don't understand what this was for, but the tests pass without it.

That doesn't mean the tests are correct, but I don't understand the problem being solved by this removed line.
Need to figure out if that is a good idea or if there is an alternative way to test this functionalitty that does not require admin.
I'm not sure of the utility of these tests.

 1. Dozens of them fail on my machine.
 2. The tests that get run will vary from machine to machine because of the way the tests are built.
 3. I think that maybe more tests the other libraries on the system rather than tests comtypes itself.
15+ years ago Thomas Heller created a test running system that could enable or disable all sorts of different tests based upon various strings in this array.

We have better solutions for this nowadays which the test suite should evolve to use.  For now we'll use the `*` to enable all tests in this bespoke test running system.
These need investigated in more detail.
Needs further investigation.
The L suffix doesn't matter for python2, but causes python3 to not parse the file.
Both python2.7 and 3 support the b"x" format.
If this test is necessary we should introduce dev dependencies to comtypes.
Probably should move this to github issues once it's fleshed-out some.
@dmwyatt dmwyatt mentioned this pull request Feb 6, 2022
@vasily-v-ryabov
Copy link
Collaborator

I understand how important to support your open source work to keep motivated. In my mind I would wish to improve not only comtypes, but also libffi which doesn't support union by value in a structure: libffi/libffi#33 but it requires too much time for my current load. So the priority for it is very low. I have too many wishes and too few results in open source for last 2-3 years. So please always point me to phrases that create not very good impression. I'm open for self-improvements.

@kdschlosser
Copy link
Contributor

One of the larger reasons why comtypes needs to have a full battery of unit tests is not so much to check comtypes but it is to check Python. Now I know you are familiar with what happened in a release of Python 3.9 when for some reason someone has decided to not allow Unions to be passed to a c function. One person came across a very specific bug and testing to confirm the bug was only done on Linux and the bug didn't even exist in Python but it existed in some dependency. it broke comtypes. I was one of the first people that brought the issue up to the Python developers and you were right there confirming that they broke something.

the removal of IE in Windows 11 I think is the first time Microsoft has EVER removed something they deprecated. There are API's in Windows that date back to Windows 3.10 and have been deprecated since Windows 95 came out but yet they are still available and they still work to this day in Windows 11.

Any tests that use IE need to be updated and an alternate way to test needs to be done and it needs to be a test that is not dependent on using a 3rd party application. Tests that use Excel should also be changed.

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Feb 17, 2022

FYI. pywinauto has some tests OS dependent as well. But they pass 100% on AppVeyor for a long time. So making them OS independent is low priority.

It's important that they run on AppVeyor, but, IMO, it's also important that they run on developers machines.

I'm way less likely to contribute to a project where the code->test->code-some-more loop is interrupted with me having to push to GitHub over and over to get the tests run. I'm also way less likely to contribute if how to run the tests isn't well documented or if they require a lot of local setup.

This isn't to say we shouldn't have some tests that maybe require some compiling or something, but the default testing experience should be easy and fast. Tests that require specific, not-standard, stuff in the environment should be in a separate test suite. It's not too difficult to create multiple test suites with unittest.

BTW, I'm still working on this, just been otherwise busy recently...I'll be back on it in full(er) force shortly.

There are API's in Windows that date back to Windows 3.10 and have been deprecated since Windows 95 came out but yet they are still available and they still work to this day in Windows 11.

Funnily enough, I started on a new commercial project this year that absolutely depends on a deprecated Windows API and I'm not too worried about it disappearing anytime soon.

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Feb 17, 2022

Unless disagreed with, I'm planning on getting this running on python2, python3, and AppVeyor and then considering this PR done (I'll see what I can pull in from @kdschlosser's work if that's OK. May be that there isn't much in the way of conflicts and his PR can be merged separately). At that point, I'd say it should be merged. I'll then open an issue (separate issues maybe?) with all the tests that have been skipped and then various people can triage those.

@kdschlosser
Copy link
Contributor

Let me tear apart the MSVC related code to trim it down so it's only what is needed to set up the build environment for the midl compiler. This is needed for the server tests instead of just blindly creating a subprocess and expecting the user to have a build environment set up before running the tests.

I will make a fork of your branch and submit PRs to that branch and when you merge those PRs they will get added to this PR. This way if you want additional help sorting things out we can do it on your fork and we can hash out what part is being worked on by you or I.

Ideally I would like to have 100% working test suite with nothing skipped. I think between the 2 of us we should be able to accomplish this pretty easily. What is going to take the most amount of work is locating a replacement for Excel and IE and whatever else is used that is not built into Windows. I think we should be able to find replacement "factory" applications that have COM objects that we will be able to use.

I made issues #280 and #256 that should help out a great deal when locating compatible COM objects. I have a script that I wrote that enumerate all interfaces and typelibs in Windows that should also be helpful.

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Feb 18, 2022

@kdschlosser That sounds good. To be honest, I am not a Windows programmer, I just have a need to access a win32 API from time to time (and usually regret it because I don't really want to learn it well!). It sounds like you'd be much more qualified to get a lot of these tests working correctly and to assess how difficult it would be to get them running.

I'm mainly concerned with something I see happen often and I want to account for it: Someone comes along with a good contribution to some open source project but it peters out because they run out of time or whatever.

I try to structure my contributions so that smaller parts of what I'd like my overall contribution to be are usable if I get too busy (or, to be honest, too uninterested!) to do everything I'd like to.

Thus my focus on a usable test suite ASAP that we can then build upon. This gives an opportunity for others to contribute in an easier manner and then if I disappear the project is left in a better state then when I started.

@kdschlosser
Copy link
Contributor

This is definitely right up your alley then.

https://github.com/kdschlosser/pyWinAPI

That project is not something that you would run but it is used more as a reference and to copy and past from. Makes those uninteresting and boring parts of projects go by faster. LOL

If you don't mind me asking what is your programming forte??

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Feb 20, 2022

If you don't mind me asking what is your programming forte??

I'm very much a generalist. Current projects involve:

  • a smaller project in the vein of pywinauto...albeit with a different focus
  • a desktop app for managing backups on Windows
  • pursuing a job building data pipelines
  • pursuing a job building frontend/backend for a company in the architecture/engineering/construction (AEC) space
  • building a project for analyzing the used automobile market. lots of web scraping, computer vision, and web backend/frontend work

In other news I'm hoping to dig back in to this PR later this week...

pyWinAPI looks like a useful resource!

@kdschlosser
Copy link
Contributor

I have found it really useful. It's nice to be able to copy and paste from, especially when needing a monster sized structure or an interface with a lot of methods.

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Mar 6, 2022

I've got test suite running on 2.7 and 3.10.

@vasily-v-ryabov Want me to add the suite to AppVeyor?

edit: I added them.

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Mar 6, 2022

Tests work fine locally, but lots of failures on AppVeyor. Maybe something to do with the build environment? I don't quite understand what is happening, so if someone could look at the errors and give me a pointer?

@dmwyatt
Copy link
Contributor Author

dmwyatt commented Mar 6, 2022

For posterity's sake here's one of the errors on AppVeyor that is repeated multiple times:

======================================================================
ERROR: test_wmi (test_wmi.Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\comtypes\comtypes\test\test_wmi.py", line 22, in test_wmi
    wmi = CoGetObject("winmgmts:")
  File "C:\projects\comtypes\comtypes\client\__init__.py", line 264, in CoGetObject
    return _manage(punk,
  File "C:\projects\comtypes\comtypes\client\__init__.py", line 188, in _manage
    obj = GetBestInterface(obj)
  File "C:\projects\comtypes\comtypes\client\__init__.py", line 110, in GetBestInterface
    mod = GetModule(tlib)
  File "C:\projects\comtypes\comtypes\client\_generate.py", line 159, in GetModule
    mod = _CreateWrapper(tlib, pathname)
  File "C:\projects\comtypes\comtypes\client\_generate.py", line 227, in _CreateWrapper
    generate_module(tlib, ofi, pathname)
  File "C:\projects\comtypes\comtypes\tools\tlbparser.py", line 767, in generate_module
    gen.generate_code(list(items.values()), filename=pathname)
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 290, in generate_code
    self.generate_all(items)
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 206, in generate_all
    self.generate(item)
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 202, in generate
    mth(item)
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 882, in ComInterface
    self.generate(itf.get_head())
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 202, in generate
    mth(item)
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 903, in ComInterfaceHead
    self.generate(base.get_head())
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 202, in generate
    mth(item)
  File "C:\projects\comtypes\comtypes\tools\codegenerator.py", line 798, in External
    comtypes.client.GetModule(ext.tlib)
  File "C:\projects\comtypes\comtypes\client\_generate.py", line 159, in GetModule
    mod = _CreateWrapper(tlib, pathname)
  File "C:\projects\comtypes\comtypes\client\_generate.py", line 242, in _CreateWrapper
    mod = _my_import(fullname)
  File "C:\projects\comtypes\comtypes\client\_generate.py", line 26, in _my_import
    return __import__(fullname, globals(), locals(), ['DUMMY'])
  File "C:\projects\comtypes\comtypes\gen\_00020430_0000_0000_C000_000000000046_0_2_0.py", line 18, in <module>
    OLE_YSIZE_PIXELS = c_int
NameError: name 'c_int' is not defined

@vasily-v-ryabov
Copy link
Collaborator

Something is broken between 1.1.11 and 1.1.12. @kdschlosser FYI.

@junkmd
Copy link
Collaborator

junkmd commented Mar 17, 2022

Broken part is provably here.
#284 (comment)

@vasily-v-ryabov
Copy link
Collaborator

@junkmd's PR was merged. After rebasing this PR from master branch, 64-bit comtypes tests are passing, 32-bit tests are not: https://ci.appveyor.com/project/pywinauto/comtypes/builds/43693283

Comment on lines +11 to +14
if sys.version_info[0] == 3:
from importlib import reload
elif sys.version_info[0] == 2:
from imp import reload
Copy link
Collaborator

Choose a reason for hiding this comment

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

import.reload is New in version 3.4.
https://docs.python.org/3.5/library/importlib.html#importlib.reload

To py33-compatible, use from imp import reload.
https://docs.python.org/3/library/imp.html#module-imp

if sys.version_info >= (3, 4):
    from importlib import reload
else:
    from imp import reload

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or we can drop Python 3.3 support in AppVeyor matrix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or we can drop Python 3.3 support in AppVeyor matrix.

It sounds better than changing this condition.

Copy link
Collaborator

Choose a reason for hiding this comment

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

However, it is premature to drop Python X.X.

I think that this PR(by @dmwyatt) goal is not dropping Python X.X from AppVeyor matrix and supports.

The goal as is it's title, Make test suite runnable.

We should discuss dropping versions in another scope, not in this PR.

@junkmd
Copy link
Collaborator

junkmd commented May 30, 2022

@vasily-v-ryabov

32-bit tests are not: https://ci.appveyor.com/project/pywinauto/comtypes/builds/43693283

AppVeyor CI was failed in test_LoadTypeLibEx.

This problem was reported(#40) a long time ago (in 2014), but it is not resolved.

This test uses ieframe.dll or shdocvw.dll.

# IE 6 uses shdocvw.dll, IE 7 uses ieframe.dll
if os.path.exists(os.path.join(os.environ["SystemRoot"],
"system32", "ieframe.dll")):
dllname = "ieframe.dll"
else:
dllname = "shdocvw.dll"

Will it be also failed even if uses other dll?

@junkmd
Copy link
Collaborator

junkmd commented Jun 1, 2022

@dmwyatt
@vasily-v-ryabov

It seems to me that the conditional branch of the DLL used for the test is causing failure.

To change the dll for test and remove the conditional branch, comtypes/comtypes/test/test_typeinfo.py may be successful in AppVeyor CI.

I suggest this because scrrun.dll maybe non-environment-specific.

    class Test(unittest.TestCase):
        # No LoadTypeLibEx on windows ce
        def test_LoadTypeLibEx(self):
            dllname = "scrrun.dll"
            self.assertRaises(WindowsError, lambda: LoadTypeLibEx("<xxx.xx>"))
            tlib = LoadTypeLibEx(dllname)
            self.assertTrue(tlib.GetTypeInfoCount())
            tlib.GetDocumentation(-1)
            self.assertEqual(tlib.IsName("idictionary"), "IDictionary")
            self.assertEqual(tlib.IsName("IDICTIONARY"), "IDictionary")
            self.assertTrue(tlib.FindName("IDictionary"))
            self.assertEqual(tlib.IsName("Spam"), None)
            tlib.GetTypeComp()

            attr = tlib.GetLibAttr()
            info = attr.guid, attr.wMajorVerNum, attr.wMinorVerNum
            other_tlib = LoadRegTypeLib(*info)
            self.assertEqual(tlib, other_tlib)

    ##         for n in dir(attr):
    ##             if not n.startswith("_"):
    ##                 print "\t", n, getattr(attr, n)

            for i in range(tlib.GetTypeInfoCount()):
                ti = tlib.GetTypeInfo(i)
                ti.GetTypeAttr()
                tlib.GetDocumentation(i)
                tlib.GetTypeInfoType(i)

                c_tlib, index = ti.GetContainingTypeLib()
                self.assertEqual(c_tlib, tlib)
                self.assertEqual(index, i)

            guid_null = GUID()
            self.assertRaises(COMError, lambda: tlib.GetTypeInfoOfGuid(guid_null))

            self.assertTrue(tlib.GetTypeInfoOfGuid(GUID("{42C642C1-97E1-11CF-978F-00A02463E06F}")))

            path = QueryPathOfRegTypeLib(*info)
            path = path.split("\0")[0]
            self.assertTrue(path.lower().endswith(dllname))

@junkmd
Copy link
Collaborator

junkmd commented Jun 10, 2022

This PR has taken over to #298, and has been merged.

Thank you to author(@dmwyatt) and participants (@vasily-v-ryabov, @kdschlosser).

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.

None yet

4 participants