Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Adding System.Drawing.Common.Drawing2D PathGradientBrush tests. #20711 #22199

Merged
merged 6 commits into from Jul 14, 2017

Conversation

KostaVlev
Copy link

@KostaVlev KostaVlev commented Jul 13, 2017

Converting mono PathGradientBrush tests and adding few new. #20711

@hughbe
Copy link

hughbe commented Jul 13, 2017

Looks great!
I may be missing it but are there any tests for dispose here? It'd be good to make sure that ArgumentExceptions are thrown when invoking the native methods after disposal.
This is because some implementations (e.g mono's libgdiplus) may be missing argument validation for the "this" pointer and cause access violations

@KostaVlev
Copy link
Author

KostaVlev commented Jul 13, 2017

I will add test for Disposed.

Copy link
Contributor

@mellinoe mellinoe left a comment

Choose a reason for hiding this comment

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

Looks good. I left a few comments that should be fixed up before merging. Thanks for the contribution!

private readonly RectangleF _defaultRectangle = new RectangleF(1, 2, 19, 28);

[ConditionalFact(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotWindowsNanoServer))]
public void Ctor_Points_ReturmsExpected()
Copy link
Contributor

Choose a reason for hiding this comment

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

type: Returms Returns


[ConditionalTheory(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotWindowsNanoServer))]
[MemberData(nameof(WrapMode_TestData))]
public void Ctor_PointsWrapMode_ReturmsExpected(WrapMode wrapMode)
Copy link
Contributor

Choose a reason for hiding this comment

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

type: Returms Returns

[ConditionalTheory(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotWindowsNanoServer))]
[InlineData(0)]
[InlineData(1)]
public void Ctor_PointsLenghtLessThenTwo_ThrowsOutOfMemoryException(int pointsLeght)
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Lenght Length

Copy link
Contributor

Choose a reason for hiding this comment

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

Also pointsLeght pointsLength

{
using (PathGradientBrush brush = new PathGradientBrush(_defaultFloatPoints))
{
AssertExtensions.Throws<ArgumentNullException>("source",() => brush.Blend = new Blend() { Factors = new float[0], Positions = null });
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space after "source",

{
using (PathGradientBrush brush = new PathGradientBrush(_defaultFloatPoints))
{
Assert.Throws<NullReferenceException>(() => brush.InterpolationColors = new ColorBlend() { Colors = null, Positions = null });
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Some of these lines are a bit long. Can you put some of these onto multiple lines, if they are over ~120 characters?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's mainly the calls to Assert.Throws that get pretty long here.

@@ -82,7 +82,7 @@ public void Ctor_PointsNull_ThrowsArgumentNullException()
[ConditionalTheory(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotWindowsNanoServer))]
[InlineData(0)]
[InlineData(1)]
public void Ctor_PointsLenghtLessThenTwo_ThrowsOutOfMemoryException(int pointsLeght)
public void Ctor_PointsLengthLessThenTwo_ThrowsOutOfMemoryException(int pointsLeght)
Copy link
Contributor

Choose a reason for hiding this comment

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

The parameter here is still misspelled: "pointsLeght".

Copy link
Author

Choose a reason for hiding this comment

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

Ughh... done :)

@@ -82,12 +82,12 @@ public void Ctor_PointsNull_ThrowsArgumentNullException()
[ConditionalTheory(nameof(PlatformDetection) + "." + nameof(PlatformDetection.IsNotWindowsNanoServer))]
[InlineData(0)]
[InlineData(1)]
public void Ctor_PointsLengthLessThenTwo_ThrowsOutOfMemoryException(int pointsLeght)
public void Ctor_PointsLengthLessThenTwo_ThrowsOutOfMemoryException(int pointsLegth)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not trying to be picky... but it's still misspelled 😄

Copy link
Author

Choose a reason for hiding this comment

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

ha ha omg :))) I am fixing it

@karelz karelz modified the milestone: 2.1.0 Jul 14, 2017
@mellinoe
Copy link
Contributor

LGTM. Thanks!

@mellinoe mellinoe merged commit 04598fd into dotnet:master Jul 14, 2017
pjanotti pushed a commit to pjanotti/corefx that referenced this pull request Jul 20, 2017
dotnet#22199)

* Adding System.Drawing.Common.Drawing2D PathGradientBrush tests. #20711

* Adding test for Dispose().

* Fixing some spellings and splinting long statement to multiple lines.

* Completing tests for Disposed.

* Spelling correction.

* Spelling correction.
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…t/corefx#20711 (dotnet/corefx#22199)

* Adding System.Drawing.Common.Drawing2D PathGradientBrush tests. dotnet/corefx#20711

* Adding test for Dispose().

* Fixing some spellings and splinting long statement to multiple lines.

* Completing tests for Disposed.

* Spelling correction.

* Spelling correction.


Commit migrated from dotnet/corefx@04598fd
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants