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

AppVeyor testing on Windows has stopped working #2204

Closed
martinholmer opened this issue Jan 22, 2019 · 8 comments
Closed

AppVeyor testing on Windows has stopped working #2204

martinholmer opened this issue Jan 22, 2019 · 8 comments

Comments

@martinholmer
Copy link
Collaborator

About 25 months ago, @talumbau introduced in PR #1111 the execution of pytest on Windows for each GitHub pull request using the AppVeyor service. The testing has worked ever since. It is far slower than the Travis-CI execution of pytest on Linux; so much so that sometimes the development process is slowed down waiting for AppVeyor to run the tests. But it has worked.

A few days ago, it stopped working. The Travis-CI tests under both Python 3.6 and 3.7 continue to work fine.

After spending some time trying to diagnose and fix this problem, I've concluded that I don't know nearly enough about how AppVeyor works to fix this problem. For example, I can't even figure out how to make it use Python 3.6 to run the tests. It seems to want to use the latest Python 3.7 version. Look at the AppVeyor test failures for Tax-Calculator pull request #2203 for an example of the (cryptic) messages being generated over the past few days since this problem appeared.

If this problem can't be fixed soon, I vote to drop the whole AppVeyor service and rely on the Travis-CI service.

@MattHJensen @hdoupe

@hdoupe
Copy link
Collaborator

hdoupe commented Jan 24, 2019

@martinholmer This seems like another fun build error. It looks like the issue comes up when bokeh imports PIL:

taxcalc\parameters.py:11: in <module>
    from taxcalc.utils import read_egg_json, json_to_dict
taxcalc\utils.py:17: in <module>
    import bokeh.io as bio
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\io\__init__.py:51: in <module>
    from .export import export_png
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\io\export.py:34: in <module>
    from ..embed import file_html
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\embed\__init__.py:54: in <module>
    from .server import server_document
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\embed\server.py:30: in <module>
    from ..resources import DEFAULT_SERVER_HTTP_URL
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\resources.py:42: in <module>
    from .model import Model
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\model.py:33: in <module>
    from .core.properties import Any, Dict, Instance, List, String
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\core\properties.py:255: in <module>
    from .property.dataspec import AngleSpec; AngleSpec
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\core\property\dataspec.py:40: in <module>
    from .visual import FontSize, MarkerType
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\bokeh\core\property\visual.py:29: in <module>
    import PIL.Image
C:\Miniconda36-x64\envs\taxcalc-dev\lib\site-packages\PIL\Image.py:94: in <module>
    from . import _imaging as core
E   ImportError: DLL load failed: The specified module could not be found.

I found this issue in the PIL repo: python-pillow/Pillow#2945

It seems like people found multiple solutions to the problem:

  • conda update conda (we already do this in the .travis.yml script)
  • conda update --all ( we do conda conda env update -f environment.yml which may be the same thing)
  • pip install pillow (I've found that pip installs some packages better than conda. So this might work)
  • conda install --channel conda-forge pillow=5 (not sure why this would work, but it's listed as a solution in that issue)

Fixing these build errors seems like more art than science most of the time. I hope one of these commands resolves the issue. If they aren't successful, we could remove the windows build while we are still digging into the problem. We have a good number of users at AEI who are using windows. So, it's nice to know that Tax-Calculator is passing the tests on a windows machine.

@martinholmer
Copy link
Collaborator Author

@hdoupe said:

It looks like the issue comes up when bokeh imports PIL:
...
I found this issue in the PIL repo: python-pillow/Pillow#2945

@hdoupe, thanks for the diagnosis. It seem like the Anaconda people haven't been able to resolve this problem in over a year!

@martinholmer
Copy link
Collaborator Author

@hdoupe said in issue #2204:

If [these suggested fixes] aren't successful, we could remove the windows build while we are still digging into the problem. ... So, it's nice to know that Tax-Calculator is passing the tests on a windows machine.

I don't understand this line of thinking. If the tests run smoothly on a Mac under Python 3.6 and run smoothly on Linux user both Python 3.6 and Python 3.7, why would there be a problem with Tax-Calculator code running under Windows?

@hdoupe also said:

We have a good number of users at AEI who are using windows.

I'm surprised to hear this. There has not been even one download of Windows taxcalc packages for either version 0.23.3 or 0.24.0 (the two most recent versions). If you're correct, then nobody has told them they need to keep current. The documentation makes that clear. So if you are correct, the "good number of" AEI users either don't read the documentation or don't follow the procedures it recommends.

@martinholmer
Copy link
Collaborator Author

@hdoupe said in Tax-Calculator issue #2204:

it's nice to know that Tax-Calculator is passing the tests on a windows machine.

Yes, it would be "nice to know" is the costs of running the Windows tests were less than the benefits of knowing for certain.

But running Windows tests does not seem to be a PSL requirement. Besides Tax-Calculator and Behavioral-Responses, there are now two other "PSL cataloged" models: OG-USA and B-Tax, neither of which run Windows tests under AppVeyor.

Because the costs have recently risen substantially and the benefits are marginal and I don't have the knowledge or time to fix this problem, I think I'll just remove the AppVeyor service from the Tax-Calculator and Behavioral-Responses repositories.

@MattHJensen, are you OK with this plan?

@hdoupe
Copy link
Collaborator

hdoupe commented Jan 24, 2019

@martinholmer Dropping the windows appveyor build makes since to me given these issues. I think the most common problem for writing python code that is compatible with windows, mac, and linux is keeping the different file path separators straight. Tax-Calculator uses the os.path module for building up the paths. So, I don't see any problems there--it's just something to keep in mind.

@MattHJensen
Copy link
Contributor

Removing AppVeyor makes sense to me.

@martinholmer
Copy link
Collaborator Author

@hdoupe said in Tax-Calculator issue #2204:

Dropping the windows appveyor [testing] makes sense to me given these issues. I think the most common problem for writing python code that is compatible with windows, mac, and linux is keeping the different file path separators straight. Tax-Calculator uses the os.path module for building up the paths. So, I don't see any problems there--it's just something to keep in mind.

@hdoupe, thanks for you thoughts on this.

@martinholmer
Copy link
Collaborator Author

A week after opening this issue AppVeyor began working again (on 2019-01-29) without any changes in the the code in this repository. This suggests what many have suspected: that AppVeyor is not only very slow but a bit flaky.

@MattHJensen @hdoupe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants