Skip to content

infrastrucutre test cases#1074

Closed
samarthgupta1 wants to merge 2 commits intosimplcommerce:masterfrom
samarthgupta1:infrastructure_test_cases
Closed

infrastrucutre test cases#1074
samarthgupta1 wants to merge 2 commits intosimplcommerce:masterfrom
samarthgupta1:infrastructure_test_cases

Conversation

@samarthgupta1
Copy link

I have written remaining test cases for infrastructure module and some for Cms module1:

  1. test/SimplCommerce.Infrastructure.Tests/CurrencyHelperTests.cs
  2. test/SimplCommerce.Infrastructure.Tests/ReflectionHelperTests.cs
  3. test/SimplCommerce.Module.Cms.Tests/Controllers/MenuApiControllerTests.cs

@samarthgupta1
Copy link
Author

cannot merge it

Copy link
Member

@hishamco hishamco left a comment

Choose a reason for hiding this comment

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

Please format and remove unnecessary comments

{
"ConnectionStrings": {
"DefaultConnection": "Server=.;Database=SimplCommerce;Trusted_Connection=True;TrustServerCertificate=true;MultipleActiveResultSets=true"
"DefaultConnection": "server=DotNetFSD\\SQLEXPRESS; database=SimplCommerce; user id=sa; password=pass@123;trustservercertificate=true",
Copy link
Member

Choose a reason for hiding this comment

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

Revert this change

@@ -0,0 +1 @@
 No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Empty?!!

Comment on lines +8 to +10
using SimplCommerce.Infrastructure.Helpers;


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
using SimplCommerce.Infrastructure.Helpers;
using SimplCommerce.Infrastructure.Helpers;

}

[Fact]
public void IsZeroDecimalCurrencies_WithUnknownCurrency_ReturnsFalse()
Copy link
Member

Choose a reason for hiding this comment

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

You could use Theory for the above tests instead of Fact

Assert.False(result);
}
[Fact]
public void IsZeroDecimalCurrencies_WithUnknown2Currency_ReturnsFalse()
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

Assert.False(result);

}

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra whitespace

Assert.True(createdMenu.IsPublished);
}


Copy link
Member

Choose a reason for hiding this comment

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

Whitespaces!!

using Xunit;

namespace SimplCommerce.Module.Core.Tests.Components
/*namespace SimplCommerce.Module.Core.Tests.Components
Copy link
Member

Choose a reason for hiding this comment

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

??

@KaranChadha10
Copy link

@samarthgupta1 I had resolved all the comments and rebased of your PR but I didn't had access to write in you fork so I have created a new PR with all the resolved comments addressed and added the method documentation also. Attaching the PR link:
#1128. @hishamco you can review my PR and let me know if anything is left, thanks.

@hishamco
Copy link
Member

hishamco commented Apr 3, 2025

Closing this in favor of #1128.

@hishamco hishamco closed this Apr 3, 2025
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