8000 Add more domain reload tests by BadSingleton · Pull Request #1269 · pythonnet/pythonnet · GitHub
[go: up one dir, main page]

Skip to content

Add more domain reload tests #1269

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

Conversation

BadSingleton
Copy link
Contributor

What does this implement/fix? Explain your changes.

This addresses multiple issues with domain reloading and changing the underlying serialized objects by Pythonnet (e.g.: removing/renaming a member, class, namespace,..)

This also adds isolated tests for domain reloading. Each test scenario is run in its own process.

Does this close any currently open issues?

This addresses #1250, and could also address part of #1268

Any other comments?

Checklist

Check all those that are applicable and complete.

  • [* ] Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • [* ] Add yourself to AUTHORS
  • Updated the CHANGELOG

BadSingleton and others added 7 commits October 19, 2020 16:15
Domain reload tests should be isolated from each other as much as
possible. This framework dynamically creates sub-runners, assemblies,
and python modules to do the domain load-unload in an isolated way.
As each test requires it's own subprocess, we cannot use nUnit. Leverage
pytest to run each subprocess instead.
Just retrieve the object and print it.
This fixes the error in bug pythonnet#1250 by separately serializing the FieldInfo.

Probably this can be optimized a bunch. I haven't verified the performance
implications at all.
Adds MaybeSerialize<T> which handles the reload nicely.

Shows how to use it in FieldObject; it's very minimal changes to FieldObject to add this support.

This is simpler than the proof of concept code.
@dnfadmin
Copy link
dnfadmin commented Oct 28, 2020

CLA assistant check
All CLA requirements met.

Comment on lines +103 to +105
catch(SerializationException _)
{
}
Copy link
Member

Choose a reason for hiding this comment

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

What is the scenario when this could happen?

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.

3 participants
0