8000 Adds test cases for member changes during a domain reload by BadSingleton · Pull Request #1275 · pythonnet/pythonnet · GitHub
[go: up one dir, main page]
Skip to content

Adds test cases for member changes during a domain reload #1275

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

Adds test cases for member changes during a domain reload #1275

wants to merge 8 commits into from

Conversation

BadSingleton
Copy link
Contributor
@BadSingleton BadSingleton commented Nov 9, 2020 •
edited
Loading

The tests are marked as expected failures for now.

What does this implement/fix? Explain your changes.

This adds a test harness/framework for renaming/removing members during domain reload. Actual fixes for these tests will com in other pull request(s).

Does this close any currently open issues?

This will help start addressing #1250

Any other comments?

This supersedes, in part, #1269

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
Adds test cases for member changes during a domain reload
fc7da1d 
The tests are marked as expected failures for now.
@dnfadmin
Copy link
dnfadmin commented Nov 9, 2020 •
edited
Loading

CLA assistant check
All CLA requirements met.

@codecov-io
Copy link
codecov-io commented Nov 9, 2020 •
edited
Loading

Codecov Report

Merging #1275 (3adc559) into master (182faed) will decrease coverage by 3.43%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1275      +/-   ##
==========================================
- Coverage   87.97%   84.53%   -3.44%     
==========================================
  Files           1        1              
  Lines         291      291              
==========================================
- H
8000
its          256      246      -10     
- Misses         35       45      +10
Flag Coverage Δ
setup_linux 59.45% <ø> (-5.85%) ⬇️
setup_windows 74.22% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
setup.py 84.53% <0.00%> (-3.44%) ⬇️

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 182faed...3adc559. Read the comment docs.

@BadSingleton BadSingleton mentioned this pull request Nov 19, 2020
Merged
4 tasks
lostmsu
lostmsu requested changes Nov 26, 2020
View reviewed changes
Copy link
Member
@lostmsu lostmsu left a comment

Choose a reason for hiding this comment

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

What's the status of the time spent running these tests? Are they only enabled for supported configurations?

src/domain_tests/Python.DomainReloadTests.15.csproj Outdated
Comment on lines 51 to 60
<PropertyGroup Condition="$(Configuration.Contains('Debug')) AND '$(TargetFramework)'=='netstandard2.0'">
<DebugSymbols>true</DebugSymbols>
<Optimize>false</Optimize>
<DebugType>full</DebugType>
</PropertyGroup>
<PropertyGroup Condition="$(Configuration.Contains('Release')) AND '$(TargetFramework)'=='netstandard2.0'">
<DebugSymbols>true</DebugSymbols>
<Optimize>true</Optimize>
<DebugType>portable</DebugType>
</PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

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

A lot of stuff in this file seems like unnecessary clutter. Can you clean it up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's mostly a copy from another project file. I'll see what I can remove

pythonnet.sln Outdated
Comment on lines 198 to 212
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.DebugWinPY3|x64.Build.0 = DebugWinPY3|x64
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.DebugWinPY3|x86.ActiveCfg = DebugWinPY3|x86
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.DebugWinPY3|x86.Build.0 = DebugWinPY3|x86
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseMono|x64.ActiveCfg = ReleaseMono|x64
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseMono|x86.ActiveCfg = ReleaseMono|x86
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseMonoPY3|x64.ActiveCfg = ReleaseMonoPY3|x64
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseMonoPY3|x86.ActiveCfg = ReleaseMonoPY3|x86
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWin|x64.ActiveCfg = ReleaseWin|x64
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWin|x64.Build.0 = ReleaseWin|x64
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWin|x86.ActiveCfg = ReleaseWin|x86
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWin|x86.Build.0 = ReleaseWin|x86
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWinPY3|x64.ActiveCfg = ReleaseWinPY3|x64
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWinPY3|x64.Build.0 = ReleaseWinPY3|x64
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWinPY3|x86.ActiveCfg = ReleaseWinPY3|x86
{7539EBD5-7A15-4513-BCB0-1D9BEF112BC2}.ReleaseWinPY3|x86.Build.0 = ReleaseWinPY3|x86
Copy link
Member

Choose a reason for hiding this comment

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

Does the new project really need all these configurations? It might just need Debug and Release. Solution configurations do not have to map 1:1 to project configurations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not. I think it does require a x86 variant lest the xplat build doesn't run. I'll see what I can remove.

@BadSingleton
Copy link
Contributor Author

What's the status of the time spent running these tests? Are they only enabled for supported configurations?

Good question. Each case takes ~2.2 seconds on my machine. Because they run through pytest, I haven't found a way to have them not run in netstandard builds (where the runner code is ifdef'd out). The solution might be to move the test harness to a nunit test

@lostmsu
Copy link
Member
lostmsu commented Nov 28, 2020

Has this been completely superseded by #1287 ?

@BadSingleton
Copy link
Contributor Author

Has this been completely superseded by #1287 ?

We can consider it has. The original plan was to have the tests merged in before the fixes, but the fixes requires the tests..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lostmsu lostmsu lostmsu requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BadSingleton @dnfadmin @codecov-io @lostmsu
0