Skip to content

Adds tests for AxSystemMonitor#10675

Merged
ricardobossan merged 7 commits into
dotnet:mainfrom
ricardobossan:Issue_8302_Add_Tests_For_Ax_Controls
Jan 22, 2024
Merged

Adds tests for AxSystemMonitor#10675
ricardobossan merged 7 commits into
dotnet:mainfrom
ricardobossan:Issue_8302_Add_Tests_For_Ax_Controls

Conversation

@ricardobossan
Copy link
Copy Markdown
Member

@ricardobossan ricardobossan commented Jan 17, 2024

Related #8302

Proposed changes

  • Instead of consolidating multiple controls into a single pull request, opted for separate PRs for each control for better organization.
  • Followed Lonitra's instructions on related issue #8294 by adding Ax control references to both the AxHosts project and the WinForms test project. Additionally, added corresponding tests.

Customer Impact

  • None

Regression?

  • No

Risk

  • Minimal

Test methodology

  • Unit tests

Test environment(s)

  • 9.0.100-alpha.1.23618.3
Microsoft Reviewers: Open in CodeFlow

@ricardobossan ricardobossan self-assigned this Jan 17, 2024
@ricardobossan ricardobossan requested a review from a team as a code owner January 17, 2024 00:08
@ricardobossan ricardobossan marked this pull request as draft January 17, 2024 00:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 17, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (fb21385) 72.53589% compared to head (daae6e1) 72.64503%.
Report is 30 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #10675         +/-   ##
===================================================
+ Coverage   72.53589%   72.64503%   +0.10914%     
===================================================
  Files           2900        2896          -4     
  Lines         622558      621803        -755     
  Branches       47003       46854        -149     
===================================================
+ Hits          451578      451709        +131     
+ Misses        162698      161793        -905     
- Partials        8282        8301         +19     
Flag Coverage Δ
Debug 72.64503% <95.74468%> (+0.10914%) ⬆️
production 43.44075% <ø> (+0.10061%) ⬆️
test 97.34194% <95.74468%> (+0.02785%) ⬆️

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

@ricardobossan ricardobossan marked this pull request as ready for review January 17, 2024 01:10
Copy link
Copy Markdown
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

Were you able to add AxSystemMonitor in the AxHosts form designer and see it during runtime?


namespace System.Windows.Forms.Tests;

public class AxSystemMonitorTests
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you implement IDisposable so that the fields can be properly disposed similar to how ErrorProviderAccessibleObjectTests does so

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@JeremyKuhne was able to investigate #10692 and looks like the error you were running into while implementing IDisposable is an issue with only the AxSystemMonitor control. In order to implement IDisposable successfully, could you also add the following to the beginning of your implementation of Dispose():

using NoAssertContext context = new();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Worked out fine now
image

@lonitra lonitra added the waiting-author-feedback The team requires more information from the author label Jan 17, 2024
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Jan 17, 2024
@ricardobossan
Copy link
Copy Markdown
Member Author

ricardobossan commented Jan 17, 2024

Were you able to add AxSystemMonitor in the AxHosts form designer and see it during runtime?

@lonitra, I successfully added AxSystemMonitor to the designer. However, upon attempting to run the AxHosts project, I encountered the following error:

AxHosts Runtime Error

I haven't figured out how to solve this issue yet and would appreciate any insights or suggestions on how to address it.

After closing VS and rebuilding, I could run it:

image


namespace System.Windows.Forms.Tests;

public class AxSystemMonitorTests
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@JeremyKuhne was able to investigate #10692 and looks like the error you were running into while implementing IDisposable is an issue with only the AxSystemMonitor control. In order to implement IDisposable successfully, could you also add the following to the beginning of your implementation of Dispose():

using NoAssertContext context = new();

@lonitra lonitra added the waiting-author-feedback The team requires more information from the author label Jan 18, 2024
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Jan 19, 2024
Copy link
Copy Markdown
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lonitra lonitra added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Jan 22, 2024
@ricardobossan ricardobossan merged commit d5528f6 into dotnet:main Jan 22, 2024
@ghost ghost added this to the 9.0 Preview1 milestone Jan 22, 2024
@ghost ghost removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Jan 22, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants