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

Failing test on Windows / Python 2.7 + 3.4 #45

Closed
matthew-brett opened this issue Feb 7, 2018 · 13 comments
Closed

Failing test on Windows / Python 2.7 + 3.4 #45

matthew-brett opened this issue Feb 7, 2018 · 13 comments

Comments

@matthew-brett
Copy link

Format of exponent in output string appears to be different from other platforms on Windows Python 2.7 and 3.4:

================================== FAILURES ===================================
__________________________ test_constraint_creation ___________________________
    def test_constraint_creation():
        """Test constraints creation and methods.
    
        """
        v = Variable('foo')
        c = Constraint(v + 1, '==')
    
        assert c.strength() == strength.required and c.op() == '=='
        e = c.expression()
        t = e.terms()
        assert (e.constant() == 1 and
                len(t) == 1 and t[0].variable() is v and t[0].coefficient() == 1)
    
>       assert str(c) == '1 * foo + 1 == 0 | strength = 1.001e+09'
E       AssertionError: assert '1 * foo + 1 ... = 1.001e+009' == '1 * foo + 1 =...h = 1.001e+09'
E         - 1 * foo + 1 == 0 | strength = 1.001e+009
E         ?                                       -
E         + 1 * foo + 1 == 0 | strength = 1.001e+09
..\kiwi\py\tests\test_constraint.py:25: AssertionError

Maybe worth a platform check in the test?

@matthew-brett matthew-brett changed the title Failing test on Windows / Python 2.7 Failing test on Windows / Python 2.7 + 3.4 Feb 7, 2018
@MatthieuDartiailh
Copy link
Member

I will try to fix it in a more robust way using regexes. Hopefully today.

@MatthieuDartiailh
Copy link
Member

Ok I just pushed a fix to #46. If Travis comes back happy and you can confirm it fixes your issue I will merge.

@matthew-brett
Copy link
Author

Passing test run for Windows / Python 2.7, #46

https://ci.appveyor.com/project/matthew-brett/kiwisolver-wheels/build/job/gq28639o08m3es0s

Passing test grid:

https://ci.appveyor.com/project/matthew-brett/kiwisolver-wheels/build/1.0.4

Are you planning to release in the near future with this fix, by any chance?

@MatthieuDartiailh
Copy link
Member

As the tests are not in the release do you need a new release or simply for me to push on master. I can make a bugfix release just for the tests, if you need it.

@matthew-brett
Copy link
Author

If you do a bugfix release, then I don't have to patch round the error in the wheel-building repo - because we're running the tests as part of the build. If not then I can put a hack in to skip the tests or apply #46 to the repo before building / testing, but I'd rather avoid that if possible.

@MatthieuDartiailh
Copy link
Member

You get the repo from the Github tag, I guess. As this release would only touch the tests an not modify what people find on PyPI, I can do a release quickly (possibly today) without uploading to PyPI. Would that work for you ?

@matthew-brett
Copy link
Author

The wheel building repo has a field in a couple of files called BUILD_COMMIT which specifies the commit to build - at the moment it's a commit hash for #46, but it can also be a tag or branch name. With the other projects I'm involved in, we try very hard to make sure that the code being built exactly corresponds to the pypi release, including tests, but if you'd rather not make a bugfix release for the tests, that's fine, we can just use the PR as a release build.

About the wheel building - this is the wheel-building repo:

https://github.com/MacPython/kiwisolver-wheels

I just got it working in the hope that y'all would consider using this repo to build wheels for upload to PyPI. I will help in whatever way you'd like. For example, I trigger builds and upload wheels for quite a few projects, but others use the instructions in the wheel-building repo to do it themselves:

https://github.com/MacPython/kiwisolver-wheels#uploading-the-built-wheels-to-pypi

I've just added you as an owner to the repository. Let me know how I can best help?

@MatthieuDartiailh
Copy link
Member

As the tests are not part of the release code currently and as neither the solver nor the bindings version would change, I don't think it is worth tagging a new release. If for the time being we can work with the commit hash (and add a note explaining why probably) this is fine.
I must say I am very interested in uploading wheels to PyPI, I will look into the process and let you know should I encounter any issue. Two related projects could actually benefit from the infrastructure of MacPorts atom and enaml (found under the same organization). I already build conda packages for them using conda-forge but never built wheels because I do not have the infrastructure to do so. Could you create repos for those two projects and add me as maintainer, I will take care of getting the builds to work ? Hopefully for those two the tests of the tags will all pass.

@matthew-brett
Copy link
Author

No problem about the release. Did you manage to upload the wheels OK? I've made a couple of repos for you and added you as an admin collaborator.

@MatthieuDartiailh
Copy link
Member

Thanks for the repos. I noticed that Travis did not yet started the builds, should I need to do anything to enable them once everything is ready ?
I had no time yet to look at the release, hopefully I will have some time tomorrow (or this week-end). Looking at the README I should create a 1.0.1 branch in the repo first no ? Thanks again with your help with this I greatly appreciate it.

@matthew-brett
Copy link
Author

You can make a branch in the README - that keeps track of what code you used to build the wheel - but there's no requirement.

I think I enabled the builds now. There are a couple of build failures for atom / Python 3.6. The enaml builds seem to need a Qt install.

@MatthieuDartiailh
Copy link
Member

Ok thanks for enabling the builds. The atom issue is something I have already seen in conda-forge conda-forge/atom-feedstock#2 and that is fairly easy to fix. Enaml can be used without qt (using another package providing a different backend) but for testing we need to have it yes. I will add it.

@MatthieuDartiailh
Copy link
Member

Ok I just uploaded the wheel. Such a simple a process ! Thanks for the tooling !
I will look into atom and enaml and let you know if I have any issues there.

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

No branches or pull requests

2 participants