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

CI: Try simple pypy3 template job add #82

Merged
merged 6 commits into from
Sep 9, 2019
Merged

CI: Try simple pypy3 template job add #82

merged 6 commits into from
Sep 9, 2019

Conversation

bskinn
Copy link
Owner

@bskinn bskinn commented Sep 8, 2019

Part of #81

@codecov-io
Copy link

codecov-io commented Sep 8, 2019

Codecov Report

Merging #82 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #82   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines         101    101           
=====================================
  Hits          101    101

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d93a361...0172faf. Read the comment docs.

@bskinn bskinn requested a review from jayvdb September 8, 2019 11:59
@bskinn
Copy link
Owner Author

bskinn commented Sep 8, 2019

@jayvdb pypy3 was easy to add. I had to relax slightly a few error message checks -- apparently pypy3 does something different with periods on the ends of error messages??

I only added a linux pypy3 job -- are the platform differences substantial enough that I should add win/mac too?

@jayvdb
Copy link
Contributor

jayvdb commented Sep 9, 2019

are the platform differences substantial enough that I should add win/mac too?

Win yes; Mac, no.

Replied longer at #81 (comment)

- pypy3 doesn't append a period to its error messages?
  And/or, it actively strips them??
- The warnings_are_errors fixture was returning None when
  '-W' was not supplied, and pytest doesn't like it when you
  do a bare 'assert None'. So, explicit bool conversion added
- Improve skip message for tests using warnings when
  '-W error::Warning' is passed
- Add Windows pypy3 job, per
  #82 (comment)
Copy link
Contributor

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

If the difference is only "I/O operation on closed file" vs "I/O operation on closed file." , it would be nice to have them as two constants at the top, and use in instead of ==

#78 is already needing a constant for the msg "underlying buffer has been detached"

@bskinn
Copy link
Owner Author

bskinn commented Sep 9, 2019

are the platform differences substantial enough that I should add win/mac too?

Win yes; Mac, no.

Replied longer at #81 (comment)

Ennnnnhhh, Azure's Windows pypy3 appears broken. :-/

@bskinn
Copy link
Owner Author

bskinn commented Sep 9, 2019

Oh, hm.... looks like a pytest problem...

@jayvdb
Copy link
Contributor

jayvdb commented Sep 9, 2019

You might also try disabling pytest's use of faulthandler.

https://github.com/vstinner/faulthandler (backport to py27, and py26 was working) says

For PyPy, faulthandler is builtin since PyPy 5.5: use pypy -X faulthandler.

That repo would also be a good spot to identify issues. We might be able to get that backport to override the PyPy native one if it is broken.

@bskinn
Copy link
Owner Author

bskinn commented Sep 9, 2019

But that's regular pypy (Python 2) .. does that also apply to pypy3?

Also increase quality of naming on the extra-cmd steps.
@bskinn
Copy link
Owner Author

bskinn commented Sep 9, 2019

pytest downgrade successful.

Might as well keep that mess separate....
bskinn added a commit that referenced this pull request Sep 9, 2019
- Eliminate "underlying buffer has been detached" magic string
- Use 'assert x in {set}' construction for accommodating the
  extra period pypy3 adds in (per
  #82 (review)
- Eliminate "underlying buffer has been detached" magic string
- Use 'assert x in {set}' construction for accommodating the
  extra period pypy3 adds in (per
  #82 (review)

Recommit to re-trigger CI.
@bskinn
Copy link
Owner Author

bskinn commented Sep 9, 2019

Ok, @jayvdb, I think it's ready.

@bskinn bskinn merged commit eb267c8 into master Sep 9, 2019
@bskinn bskinn deleted the azure-pypy3 branch September 9, 2019 03:54
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

3 participants