Skip to content

Use Core.Utilities.CopyDirectory in tests #2670

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
Merged
merged 3 commits into from
Feb 14, 2019
Merged

Use Core.Utilities.CopyDirectory in tests #2670

merged 3 commits into from
Feb 14, 2019
+158 −26

Conversation

DasSkelett
Copy link
Member

In #2627 I created a new, more robust CopyDirectory() than the existing one in TestData.

Replaced all calls to TestData.CopyDirectory() with Utilities.CopyDirectory().
Removed TestData.CopyDirectory().

All tests still run successful.

@HebaruSan HebaruSan added Easy This is easy to fix Pull request Tests Issues affecting the internal tests labels Jan 28, 2019
@HebaruSan
Copy link
Member

What about adding some new tests at Tests/Core/Utilities.cs, to make sure Utilities.CopyDirectory works by itself?

The best practices that I found for tests were:

  1. Name the functions as MethodTested_Scenario_DesiredOutcome
  2. Organize the code into three sections:
    • // Arrange, setting up the test data
    • // Act, performing the step that we want to test
    • // Assert, checking that it turned out OK

Not all the old tests follow this pattern, but the new ones should.

@DasSkelett
Copy link
Member Author

Added some tests. Any more ideas of scenarios I could test for? Something I forgot?

To compare files, I added a new helper function and wrote two tests for that one too. (Testception...)

Sorry, something went wrong.

@politas
Copy link
Member

politas commented Feb 10, 2019 •
edited
Loading 

Errors, Failures and Warnings

1) Failed : Tests.Core.Utilities.CopyDirectory_DestNotEmpty_ThrowsException
  Expected: <System.IO.IOException>
  But was:  <CKAN.PathErrorKraken: Directory not empty: 
  at CKAN.Utilities._CopyDirectory (System.String sourceDirPath, System.String destDirPath, System.Boolean copySubDirs) [0x00066] in <04d8855fbe4e4a4882235b62af18aa2d>:0 
  at CKAN.Utilities.CopyDirectory (System.String sourceDirPath, System.String destDirPath, System.Boolean copySubDirs) [0x00008] in <04d8855fbe4e4a4882235b62af18aa2d>:0 
  at Tests.Core.Utilities.<CopyDirectory_DestNotEmpty_ThrowsException>b__7_0 () [0x00000] in <10779aeb9933418ebf2c001a31da16d2>:0 
  at NUnit.Framework.Assert.Throws (NUnit.Framework.Constraints.IResolveConstraint expression, NUnit.Framework.TestDelegate code, System.String message, System.Object[] args) [0x00034] in <64cd73ddfc8a470aa513c8243b429bdb>:0 >
  at Tests.Core.Utilities.CopyDirectory_DestNotEmpty_ThrowsException () [0x0001c] in <10779aeb9933418ebf2c001a31da16d2>:0 

Run Settings
    DisposeRunners: True
    WorkDirectory: /mnt/raid/Repositories/mono/CKAN/_build/test/nunit
    ImageRuntimeVersion: 4.0.30319
    ImageTargetFrameworkName: .NETFramework,Version=v4.5
    ImageRequiresX86: False
    ImageRequiresDefaultAppDomainAssemblyResolver: False
    NumberOfTestWorkers: 4

Test Run Summary
  Overall result: Failed
  Test Count: 680, Passed: 674, Failed: 1, Warnings: 0, Inconclusive: 0, Skipped: 5
    Failed Tests - Failures: 1, Errors: 0, Invalid: 0
    Skipped Tests - Ignored: 0, Explicit: 5, Other: 0
  Start time: 2019-02-10 02:41:40Z
    End time: 2019-02-10 02:42:21Z
    Duration: 41.175 seconds

Results (nunit3) saved as TestResult.xml
An error occurred when executing task 'Test-UnitTests+Only'.

I think that result got changed in #2665?

@DasSkelett
Assert PathErrorKraken instead of IOException
f2578a3 
PR KSP-CKAN#2665 changed the IOException to PathErrorKraken in `CopyDirectory()`.
Now the new tests for `CopyDirectory()` assert the new kraken.
@politas politas merged commit f2578a3 into KSP-CKAN:master Feb 14, 2019
politas added a commit that referenced this pull request Feb 14, 2019
@politas
Merge #2670 Use Core.Utilities.CopyDirectory in tests
62da77d
@DasSkelett DasSkelett deleted the change/useNewCopyDirectoryEverywhere branch February 18, 2019 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Easy This is easy to fix Tests Issues affecting the internal tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@DasSkelett @HebaruSan @politas