Skip to content

WI00856225 - Remove FluentAssertions from Blazor.Diagrams.Core.Tests#49

Merged
LeonamAnjos merged 15 commits intomasterfrom
Blazor/fix/WI00856225/RemoveFluentAssertions_Blazor.Diagrams.Core.Tests
Feb 4, 2025
Merged

WI00856225 - Remove FluentAssertions from Blazor.Diagrams.Core.Tests#49
LeonamAnjos merged 15 commits intomasterfrom
Blazor/fix/WI00856225/RemoveFluentAssertions_Blazor.Diagrams.Core.Tests

Conversation

@Srude-Vena
Copy link

WI0085622-FluentAssertions needs to be removed from the project Blazor.Diagrams.Core.Tests

Copy link

@LeonamAnjos LeonamAnjos left a comment

Choose a reason for hiding this comment

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

I found these two references to FluentAssertions. Should we remove them now? Or do you plan to have a second PR for that?

Blazor.Diagrams.Tests.csproj

Directory.Packages.props

Apart from that, most comments are minors.

Comment on lines 23 to 24
Assert.Equal(203.4,pt.X);
Assert.Equal(361.8, pt.Y);

Choose a reason for hiding this comment

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

Suggested change
Assert.Equal(203.4,pt.X);
Assert.Equal(361.8, pt.Y);
Assert.Equal(203.4,pt.X); // 2*X + panX + left
Assert.Equal(361.8, pt.Y); // 2*Y + panY + top

Let's keep this comment. It may be useful when debugging.

Comment on lines 39 to 42
Assert.Equal(10, node.Position.X);
Assert.Equal(0, node.Position.Y);
Assert.Equal(90, node.Size.Width);
Assert.Equal(215, node.Size.Height);

Choose a reason for hiding this comment

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

Suggested change
Assert.Equal(10, node.Position.X);
Assert.Equal(0, node.Position.Y);
Assert.Equal(90, node.Size.Width);
Assert.Equal(215, node.Size.Height);
Assert.Equal(10, node.Position.X);
Assert.Equal(0, node.Position.Y);
Assert.Equal(90, node.Size.Width);
Assert.Equal(215, node.Size.Height);

@LeonamAnjos LeonamAnjos changed the title FluentAssertion removed from chunk1 files WI00856225 - Remove FluentAssertions from Blazor.Diagrams.Core.Tests Jan 30, 2025
@Srude-Vena
Copy link
Author

I found these two references to FluentAssertions. Should we remove them now? Or do you plan to have a second PR for that?

Blazor.Diagrams.Tests.csproj

Directory.Packages.props

Apart from that, most comments are minors.

Removal of FluentAssertions from Blazor.Diagrams.Tests.csproj and [Directory.Packages.props] is part of my 2nd work item
WI00856226-FluentAssertions removal from Blazor.Diagrams.Tests by Srude-Vena · Pull Request #51 · WiseTechGlobal/Blazor.Diagrams
Above mentioned changes are made in Workitem WI00856226

Copy link

@Alvarenga1 Alvarenga1 left a comment

Choose a reason for hiding this comment

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

Let's fix the formatting and address all GitHub actions warnings.

<ItemGroup>
<PackageReference Include="SvgPathProperties" />
<ItemGroup>
<PackageReference Include="SvgPathProperties" />

Choose a reason for hiding this comment

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

This is very minor, but could we fix the spacing here ?

<Reference Include="SvgPathProperties" PrivateAssets="All">
<HintPath>..\..\packages\svgpathproperties\1.1.2\lib\netstandard2.0\SvgPathProperties.dll</HintPath>
</Reference>
<PackageReference Include="SvgPathProperties" PrivateAssets="All" />

Choose a reason for hiding this comment

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

Same here, formatting.

Copy link
Author

Choose a reason for hiding this comment

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

spaces and warnings removed and checks passed

Copy link

@LeonamAnjos LeonamAnjos left a comment

Choose a reason for hiding this comment

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

LGTM

@LeonamAnjos LeonamAnjos merged commit 88e8cab into master Feb 4, 2025
2 checks passed
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