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

Changes to make tests pass with https://github.com/zopefoundation/ZODB/pull/66 #45

Closed
wants to merge 2 commits into from

Conversation

jimfulton
Copy link
Member

You may not want to merge this as is. I had to comment out a raise in _rollback_load_connection to make checkAutoReconnect pass. This is because sync is now called when a connection is opened.

The most important change is to comment out the monkey patch, which you probably want to delete.

Also, check16MObject failed for me for mysql on master with released ZODB. Maybe this is an issue with my setup.

@jamadden
Copy link
Member

Thanks. I would prefer not to merge as-is, but I will use it as a guideline for a different PR. Specifically, I think I will catch the list of disconnected/closed exceptions to handle checkAutoReconnect and add a separate test.

Any thoughts about the test failures? I suppose it's part of the monkey-patch that got deleted. I may need to do some attribute/version checking to keep part of the patch around for existing ZODB versions.

The check16MObject thing also shows up on Travis. I've seen it locally too. It has something to do with a configuration or protocol knob for the server or client. I haven't cared enough to track it down again because apparently I dealt with it on my system some time ago so it works for me :)

@jimfulton
Copy link
Member Author

I assume the test failures are due to the missing monkey patch. This is sad (and a little surprising, since the monkey patch is pretty close to what's already in ZODB4.

@jamadden
Copy link
Member

I can try to dig into the details to come up with a definitive cause.

@jimfulton
Copy link
Member Author

Before digging to much, I'd restore the patch and see if the tests pass. I assume they will.

At which point, you'll have to do some sniffing and conditionally apply the patch.

Actually, I'll do that. Just a sec...

@jimfulton
Copy link
Member Author

That should fix the test failures.

@coveralls
Copy link

coveralls commented Jun 14, 2016

Coverage Status

Coverage decreased (-0.04%) to 76.178% when pulling b28a004 on jim-zodb5 into c3439c2 on master.

@jimfulton
Copy link
Member Author

OK, IDK WTF coveralls

@jamadden
Copy link
Member

Probably because "except ImportError:" blocks are ignored (as they're typically Py2/Py3 compatibility code).

@jamadden
Copy link
Member

How about #46 instead?

@jimfulton
Copy link
Member Author

#46 LGTM

@jimfulton jimfulton closed this Jun 14, 2016
@jimfulton jimfulton deleted the jim-zodb5 branch December 14, 2016 15:44
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