Added TestOption to change xml result file location #60

epsmae · 2016-10-12T12:27:04Z

We are running nunit test with a build server on an network android emulator.
For a more convenient access to the result file the destination file path and name should be settable. The default file location is on the app internal storage, which is more complex to access.

rprouse

Changes look good, but they need to be included on all supported platforms.

rprouse · 2016-10-12T13:15:19Z

src/tests/nunit.runner.tests.Droid/MainActivity.cs

+                CreateXmlResultFile = true,
+
+                // Choose a diffrent path for the xml result file
+                // ResultFilePath = Path.Combine(Environment.ExternalStorageDirectory.Path, Environment.DirectoryDownloads, "Nunit", "Results.xml")


This needs to be added to the iOS and UWP projects

rprouse · 2016-10-12T13:16:25Z

src/tests/nunit.runner.tests.Droid/Properties/AndroidManifest.xml

@@ -1,5 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <manifest xmlns:android="http://schemas.android.com/apk/res/android" android:installLocation="auto" package="nunit.runner.tests" android:versionCode="3" android:versionName="3.0">
 	<uses-sdk android:minSdkVersion="15" />
-	<application android:label="NUnit" android:icon="@drawable/icon"></application>
+	<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
+	<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />


Permissions need to be set for other platforms

rprouse · 2016-10-18T15:53:16Z

@epsmae are you able to make the requested changes, or would you like someone else to do the updates?

ChrisMaddock · 2016-10-19T12:02:18Z

Was adding things to the 3.2 milestone - will also throw this in. It'll also fix #58 👍

epsmae · 2016-10-19T14:08:01Z

I will do it in the next days!

ChrisMaddock · 2016-10-27T11:34:19Z

Hey @epsmae - is this ready for review? I haven't pulled it down to try it yet, but looks like it's implemented on every platform?

If it's good to go, it would be great if we can merge this before starting on #48. 😄

epsmae · 2016-10-27T11:45:54Z

@ChrisMaddock - I think so, I did a simple test on an iphone and a windows 8.1 phone. As I hardly work with Xamarin IOS/ Windows Phone I would be Grateful if somebody could have a look and do some tests.

ChrisMaddock · 2016-10-27T11:47:51Z

Great, thanks! I'll try and take a look tonight.

@rprouse, would you also mind taking a second look?

rprouse · 2016-10-27T13:35:19Z

I will also have a look. Someone will need to resolve conflicts. @epsmae if you can merge latest from master it would save us some time. If not, I can do it, but it might mean opening a new PR since you are working on a fork. You will still get credit for the changes you made though 😄

rprouse

Looks good except for minor changes that I have requested. Once it is good, do you want one of us to merge from master and resolve the conflicts or can you do it?

rprouse · 2016-10-29T14:13:51Z

src/tests/nunit.runner.tests.iOS/nunit.runner.tests.iOS.csproj

@@ -47,9 +49,10 @@
    <WarningLevel>4</WarningLevel>
    <ConsolePause>false</ConsolePause>
    <MtouchArch>ARMv7, ARM64</MtouchArch>
-    <CodesignKey>iPhone Developer</CodesignKey>
+    <CodesignKey>iPhone Developer: Christian Schmid (R65G567HMV)</CodesignKey>


This needs to be removed.

rprouse · 2016-10-29T14:14:09Z

src/tests/nunit.runner.tests.iOS/nunit.runner.tests.iOS.csproj

@@ -25,6 +25,8 @@
    <MtouchLink>None</MtouchLink>
    <MtouchDebug>true</MtouchDebug>
    <CodesignEntitlements>Entitlements.plist</CodesignEntitlements>
+    <CodesignKey>iPhone Developer: Christian Schmid (R65G567HMV)</CodesignKey>
+    <CodesignProvision>dccfb495-a21d-4e42-8f9b-09f3126395e5</CodesignProvision>


Needs to be removed.

rprouse · 2016-10-29T14:14:36Z

src/tests/nunit.runner.tests.iOS/nunit.runner.tests.iOS.csproj

    <MtouchDebug>true</MtouchDebug>
    <CodesignEntitlements>Entitlements.plist</CodesignEntitlements>
+    <CodesignProvision>dccfb495-a21d-4e42-8f9b-09f3126395e5</CodesignProvision>


I think you should revert all changes in this file.

rprouse · 2016-10-29T14:16:04Z

src/tests/nunit.runner.tests.wp81/MainPage.xaml.cs

@@ -53,7 +53,11 @@ public MainPage()
                //TcpWriterParameters = new TcpWriterInfo("192.168.0.108", 13000),

                // Creates a NUnit Xml result file on the host file system using PCLStorage library.
-                CreateXmlResultFile = false
+                CreateXmlResultFile = false,


We've removed WP81 since you started this PR. Someone will need to resolve the merge conflicts. Sorry.

rprouse · 2016-10-29T14:17:46Z

src/tests/nunit.runner.tests.Droid/MainActivity.cs

@@ -60,7 +60,7 @@ protected override void OnCreate(Bundle savedInstanceState)
                CreateXmlResultFile = true,

                // Choose a diffrent path for the xml result file
-                // ResultFilePath = Path.Combine(Environment.ExternalStorageDirectory.Path, Environment.DirectoryDownloads, "Nunit", "Results.xml")
+                ResultFilePath = Path.Combine(Environment.ExternalStorageDirectory.Path, Environment.DirectoryDownloads, "Nunit_123", "Results.xml")


Can you change Nunit_123 back?

rprouse · 2016-10-29T14:20:30Z

src/runner/nunit.runner/Services/XmlFileProcessor.cs

-                    await folder.CreateFolderAsync(segments[i + 1], CreationCollisionOption.OpenIfExists);
+#if __DROID__
+                    IFolder folder = await FileSystem.Current.GetFolderFromPathAsync(path, CancellationToken.None);
+#elif __IOS__


Since the Android and IOS code is the same, please change the first #if to #if __DROID__ || __IOS__

rprouse · 2016-10-29T14:21:38Z

src/runner/nunit.runner/Services/XmlFileProcessor.cs

+#elif __IOS__
+                    IFolder folder = await FileSystem.Current.GetFolderFromPathAsync(path, CancellationToken.None);
+#else
+                    IFolder folder = await FileSystem.Current.GetFolderFromPathAsync(path.Replace('/', '\\'), CancellationToken.None);


Just curious, did you do the path replacement because it wasn't working? I thought Windows accepted the Linux path separators, but I might be wrong.

It crashed on the windows phone so I had to replace the slashes. It also doesn't work with Uri, I always thought it doesn't matter on the platform.

ChrisMaddock · 2016-10-30T14:46:13Z

src/runner/nunit.runner/Services/TestOptions.cs

+        /// <summary>
+        /// File path for the xml result file
+        /// Default is [LocalStorage]/NUnitTestOutput/TestResults.xml
+        /// </summary>


If I'm reading right, I think this comment is out-dated, default is now just in localstorage? Looks like the behaviour was changed since the comment was written. ☺

ChrisMaddock · 2016-10-30T14:51:23Z

Sorry both, was hoping to look at this the other day, but this weeks just ran away from me! Looks good, @epsmae, I just had a minor aesthetic comment on top of what @rprouse has already added. 🙂 Will be a nice addition!

epsmae · 2016-11-02T17:07:56Z

Sorry, I did something foolish. Is it possible to reopen the pull request?

ChrisMaddock · 2016-11-02T17:14:33Z

The pull request is linked to the branch on your repo - I can't currently reopen it as There are no new commits on the epsmae:pull-request-result-filepath-option branch.

Just push the code back to your branch, and you/we should be able to reopen it. If it all goes wrong, just open up an new PR, don't worry about it. 😄

You haven't accidentally deleted the changes completely, have you? I think I still have a copy locally if so. 😄

rprouse · 2016-11-04T00:15:49Z

The changes you have up so far look good. Someone just needs to resolve merge conflicts. @epsmae can you do that, or would you like one of us to do it?

ChrisMaddock · 2016-11-06T22:37:23Z

All looks good to me - merged! Thanks @epsmae!

epsmae · 2016-11-07T06:20:41Z

@ChrisMaddock When a IPhone is connected it is listed in iTunes ( V9.1 and later). As the UIFileSharingEnabled key is set files can be transfered from / to the device within the app shared folder.

Added TestOption to change xml result file location

a3554fb

rprouse suggested changes Oct 12, 2016

View reviewed changes

ChrisMaddock added this to the 3.2 milestone Oct 19, 2016

Added Support for ios and wp81

8f894a4

rprouse suggested changes Oct 29, 2016

View reviewed changes

ChrisMaddock reviewed Oct 30, 2016

View reviewed changes

epsmae closed this Nov 2, 2016

epsmae force-pushed the pull-request-result-filepath-option branch from 8f894a4 to 3542be4 Compare November 2, 2016 16:36

Adjusted comment to implementation

55b5989

epsmae reopened this Nov 3, 2016

epsmae added 2 commits November 3, 2016 14:29

Change folder name and removed unnecessary else if

ce7276a

Reverted file

0f44421

merged conflicts

ddafa52

ChrisMaddock merged commit 0fa532e into nunit:master Nov 6, 2016

ChrisMaddock mentioned this pull request Nov 6, 2016

How do i get the result out from CreateXmlResultFile on iOS ? #58

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added TestOption to change xml result file location #60

Added TestOption to change xml result file location #60

epsmae commented Oct 12, 2016

rprouse left a comment

rprouse Oct 12, 2016

rprouse Oct 12, 2016

rprouse commented Oct 18, 2016

ChrisMaddock commented Oct 19, 2016 •

edited

Loading

epsmae commented Oct 19, 2016

ChrisMaddock commented Oct 27, 2016

epsmae commented Oct 27, 2016

ChrisMaddock commented Oct 27, 2016 •

edited

Loading

rprouse commented Oct 27, 2016

rprouse left a comment

rprouse Oct 29, 2016

rprouse Oct 29, 2016

rprouse Oct 29, 2016

rprouse Oct 29, 2016

rprouse Oct 29, 2016

rprouse Oct 29, 2016

rprouse Oct 29, 2016

epsmae Nov 2, 2016

ChrisMaddock Oct 30, 2016

ChrisMaddock commented Oct 30, 2016

epsmae commented Nov 2, 2016

ChrisMaddock commented Nov 2, 2016 •

edited

Loading

rprouse commented Nov 4, 2016

ChrisMaddock commented Nov 6, 2016

epsmae commented Nov 7, 2016

Added TestOption to change xml result file location #60

Added TestOption to change xml result file location #60

Conversation

epsmae commented Oct 12, 2016

rprouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rprouse commented Oct 18, 2016

ChrisMaddock commented Oct 19, 2016 • edited Loading

epsmae commented Oct 19, 2016

ChrisMaddock commented Oct 27, 2016

epsmae commented Oct 27, 2016

ChrisMaddock commented Oct 27, 2016 • edited Loading

rprouse commented Oct 27, 2016

rprouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisMaddock commented Oct 30, 2016

epsmae commented Nov 2, 2016

ChrisMaddock commented Nov 2, 2016 • edited Loading

rprouse commented Nov 4, 2016

ChrisMaddock commented Nov 6, 2016

epsmae commented Nov 7, 2016

ChrisMaddock commented Oct 19, 2016 •

edited

Loading

ChrisMaddock commented Oct 27, 2016 •

edited

Loading

ChrisMaddock commented Nov 2, 2016 •

edited

Loading