Custom arguments and environment variables. #33

panekzogen · 2016-09-06T09:18:28Z

Added possibility to set custom arguments and environment variables.

One bug fixed: michel-kraemer/gradle-download-task#47

qliklars · 2016-10-22T08:25:36Z

Is there a reason for closing but nor merging this PR?
We are very interested in the environment variable support that is added in this PR.
Is there any easy workaround with the current plugin?

timotei · 2016-10-22T09:27:30Z

src/main/groovy/com/ullink/gradle/nunit/NUnit.groovy

@@ -31,6 +31,9 @@ class NUnit extends ConventionTask {
    def test
    def testList

+    Map<String, Object> custom = [:]


I'd rename this to extraCommandLineArgs

timotei · 2016-10-22T09:29:49Z

Sorry for the delay on reviewing this :(

First of all, thanks for the contribution!

For the custom extra arguments, are there any specific ones? Maybe we should add them as properties in the plugin itself instead.

timotei · 2016-10-22T09:31:22Z

src/main/groovy/com/ullink/gradle/nunit/NUnit.groovy

@@ -31,6 +31,9 @@ class NUnit extends ConventionTask {
    def test
    def testList

+    Map<String, Object> custom = [:]
+    Map<String, Object> env = [:]


Can you please update the README.md sample, so everybody will know about these two properties?

panekzogen · 2016-10-24T14:25:44Z

Reasons for closing this PR:

We have limitation on Gradle version higher than 2.14 with download plugin version lower than 3.1.0 (Gradle 2.14 break michel-kraemer/gradle-download-task#49)
Increasing download plugin version caused new problem. (Failing to initialize org.apache.http.conn.ssl.SSLConnectionSocketFactory michel-kraemer/gradle-download-task#58). I have no idea how to fix it.

panekzogen · 2016-10-24T15:00:08Z

I'm interesting in /labels /noshadow /nologo /timeout /output and /xml without prefix. But in my opinion adding extra arguments field is more flexible solution.

timotei · 2016-10-24T21:32:39Z

👍 from me

gluck · 2016-10-26T08:59:50Z

I'm not fond of generic pass-through variables, as it defeats the purpose of having a plugin in the first place (if all you want is write the nunit command-line yourself, nothing will do a better job as using an Exec task directly).
For the options you mention, labels/noshadow/timeout/xml are already available, I'd say that nologo should be set by default, so only /output remains. An option could be added, though the common practice (to my knowledge) is to have UT text report in your build log rather than a separate file.

I assume these options come from migrating from a script build approach ?

gluck · 2016-10-26T09:00:41Z

(note that I upgraded gradle/download/httpclient on master and released 1.9 with it, should fix gradle-related issues)

qliklars · 2016-10-26T15:26:33Z

@gluck : What about the env var support? We are passing environment variables that control the placement of test artifacts in prebuilt C# test assemblies.
Do you have a suggestion for them?

gluck · 2016-10-26T16:38:18Z

Totally okay with that one, I missed the fact it was your pain point or I would have included it in the earlier release sorry.
Feel free to PR just that

panekzogen · 2016-10-27T13:40:56Z

Yes these options from migration.
Finally i added output option and env variables. From my point of view output option will not be superfluous.

gluck · 2016-10-27T14:59:50Z

src/main/groovy/com/ullink/gradle/nunit/NUnit.groovy

    @OutputFile
    File getTestReportPath() {
        // for the non-default nunit tasks, ensure we write the report in a separate file
-        // TODO: Do we need to supply prefix when user has specified its own report file name?
-        def reportFileNamePrefix = name == 'nunit' ? '' : name


any reason to remove this prefix ? in case you have mutliple nunit tasks in your build, they'll have the same report file if you remove it.

There is no possibility to specify name without prefix. But user still can set the same report file in own tasks.

@taomWTP Well, in the new change we won't have this prefix anymore by default. How about if the user does not specify the report file, we keep the previous behaviour? But even so, I don't see why this change is needed for this PR 🤔

Right, this change isn't needed for this PR, but it solves the problem that i mentioned above. I can open another PR for it if you want.
When making this change I was taking into account the fact that if user didn't specify report file, he doesn't really need it and we can follow the nunit default behavior that store all results in TestResult.xml.

There is another solution that keeps the previous behavior. It was written in your TODO comment. Namely, prefix won't be used when user specified his own report file name. I think this is more tolerant solution if you want to keep previous behavior. I will do this if you agree.

When making this change I was taking into account the fact that if user didn't specify report file, he doesn't really need it

I hear you, but we can't be sure we don't break lots of usages out there, that rely on the xml being named by the plugin automatically, based on the task name. That's why I'd say we should aim at keeping a backwards compat behavior if possible and if it doesn't prevent us from evolving.

@gluck @bcorne Feedback on this?

I suppose, that you will agree with new changes.

gluck · 2016-10-27T15:04:24Z

README.md

@@ -41,16 +41,24 @@ It creates a task 'nunit' that may be configured as follows:
        // optional - defaults to FALSE and termines the behavior of the task if the nunit-console.exe program exits
        // abnormally
        ignoreFailures = false
-
+        // optional - specify whether to write test case names to the output


kind'o weird to see this change as it's already on master

gluck · 2016-10-27T15:06:52Z

README.md

+        shadowCopy = true        
+        // redirect output to file
+        // additionally you can specify logFolder
+        logFileName = 'TestOutput.log'


did you consider putting a single argument (outputFile or something) for both ? reportFolder was split because it's used in other places, not sure we need the same here and it'll be simpler.

Yes, single argument will be simpler. I'll change this in the next commit.

gluck · 2016-10-27T15:07:44Z

src/main/groovy/com/ullink/gradle/nunit/NUnit.groovy

@@ -215,6 +228,7 @@ class NUnit extends ConventionTask {
        prepareExecute()

        def mbr = project.exec {
+            environment env


any idea (I didn't check) if when env is empty this will change the behavior from the previous plugin version (still inherits process env variables right ?)

clarify please

What @gluck Is saying is: if env returns empty/null, will that change the way the project.exec works? i.e.,: will it work the same as if no environment has been specific

timotei · 2016-12-17T16:59:20Z

Btw, you should rebase against latest changes, seems there are some conflicts on README.md

panekzogen · 2016-12-27T16:47:43Z

Environment method is based on Map.putAll(). That's why empty env does not change execution, but null causes NullPointerException. I fixed it.
Integration crashes on nunit 3.5 test. What will we do about it?
Sorry for a delay.

panekzogen · 2017-03-02T18:20:22Z

Why don't you merge the request?

kuniss · 2017-07-07T08:53:38Z

When do you plan to merge this pull request?
I'm also interested in having the possibility to define environment variables...

BTW, great work! Thank you for the Ullink Gradle plugins!

gluck · 2017-07-07T09:16:47Z

Long overdue, sorry !

kuniss · 2017-07-07T16:41:54Z

Great! Thanks!

I guess some one needs to push a release?
Version 1.11 is not released yet on plugins.gradle.org ...

Or is on the way? Did not see it on AppVeyor nor Travis.

ngyukman · 2017-07-08T02:04:04Z

will do a release once the fix for #42 is ready, thanks for patient

kuniss · 2017-07-11T15:19:49Z

@ngyukman I saw the release 1.11 is done on GitHub. Thank you!
When do you plan to publish it to https://plugins.gradle.org/plugin/com.ullink.nunit?

ngyukman · 2017-07-13T09:42:54Z

@kuniss sorry that we are having some problem with the release in https://plugins.gradle.org/
but you can reference to the maven repo https://bintray.com/ullink/gradle-plugins/com.ullink.gradle:gradle-nunit-plugin for now, we will fix it once we have resolved the release issue

kuniss · 2017-07-13T21:02:54Z

No problem! I will wait or use the repo dependency. Thank you, for coming back with a note! Am 13.07.2017 um 11:42 schrieb Ng Yuk Man:

…

@kuniss <https://github.com/kuniss> sorry that we are having some problem with the release in https://plugins.gradle.org/ but you can reference to the maven repo https://bintray.com/ullink/gradle-plugins/com.ullink.gradle:gradle-nunit-plugin for now, we will fix it once we have resolved the release issue — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#33 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABDoA_i48sEELrYLJG5f9IpxjTpzYb5Tks5sNeafgaJpZM4J1oO0>.

Vladislav Pastushenya and others added 3 commits August 26, 2016 17:42

Environments and custom parameters

6c7ed13

Download plugin version increased

13ff5b8

Some remarks

ff4101e

panekzogen closed this Sep 6, 2016

timotei reviewed Oct 22, 2016

View reviewed changes

timotei reopened this Oct 22, 2016

timotei reviewed Oct 22, 2016

View reviewed changes

panekzogen added 2 commits October 24, 2016 17:06

custom renamed to extraCommandLineArgs

6c3a45c

Update README.md

ec8484a

panekzogen added 8 commits October 27, 2016 13:15

Update README.md

676f13e

Update NUnit.groovy

6392fdc

Update build.gradle

9ccf8b5

report file prefix removed

638c978

output arg

f20d236

Update NUnit.groovy

ba768f1

polish

3c7a086

Update README.md

a08720c

gluck reviewed Oct 27, 2016

View reviewed changes

Single log file argument

955bd98

panekzogen added 3 commits December 27, 2016 18:22

Merge branch 'master' into master

15358c2

evn null pointer fix

0e58812

Update NUnit.groovy

0c9310b

Update README.md

78c3f54

panekzogen added 2 commits March 30, 2017 19:57

Keeping backwards compatibility for reportFileName

6679130

Keeping backwards compatibility 2

d87d8b8

gluck merged commit 8ddec4a into Itiviti:master Jul 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom arguments and environment variables. #33

Custom arguments and environment variables. #33

panekzogen commented Sep 6, 2016

qliklars commented Oct 22, 2016

timotei Oct 22, 2016

timotei commented Oct 22, 2016

timotei Oct 22, 2016

panekzogen commented Oct 24, 2016

panekzogen commented Oct 24, 2016

timotei commented Oct 24, 2016

gluck commented Oct 26, 2016

gluck commented Oct 26, 2016 •

edited

Loading

qliklars commented Oct 26, 2016

gluck commented Oct 26, 2016

panekzogen commented Oct 27, 2016

gluck Oct 27, 2016

panekzogen Oct 30, 2016

timotei Mar 3, 2017

panekzogen Mar 5, 2017

timotei Mar 7, 2017

panekzogen Mar 31, 2017

gluck Oct 27, 2016

gluck Oct 27, 2016

panekzogen Oct 30, 2016

gluck Oct 27, 2016

panekzogen Oct 30, 2016

timotei Dec 17, 2016

timotei commented Dec 17, 2016

panekzogen commented Dec 27, 2016

panekzogen commented Mar 2, 2017

kuniss commented Jul 7, 2017

gluck commented Jul 7, 2017

kuniss commented Jul 7, 2017

ngyukman commented Jul 8, 2017

kuniss commented Jul 11, 2017 •

edited

Loading

ngyukman commented Jul 13, 2017

kuniss commented Jul 13, 2017 via email

Custom arguments and environment variables. #33

Custom arguments and environment variables. #33

Conversation

panekzogen commented Sep 6, 2016

qliklars commented Oct 22, 2016

Choose a reason for hiding this comment

timotei commented Oct 22, 2016

Choose a reason for hiding this comment

panekzogen commented Oct 24, 2016

panekzogen commented Oct 24, 2016

timotei commented Oct 24, 2016

gluck commented Oct 26, 2016

gluck commented Oct 26, 2016 • edited Loading

qliklars commented Oct 26, 2016

gluck commented Oct 26, 2016

panekzogen commented Oct 27, 2016

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotei commented Dec 17, 2016

panekzogen commented Dec 27, 2016

panekzogen commented Mar 2, 2017

kuniss commented Jul 7, 2017

gluck commented Jul 7, 2017

kuniss commented Jul 7, 2017

ngyukman commented Jul 8, 2017

kuniss commented Jul 11, 2017 • edited Loading

ngyukman commented Jul 13, 2017

kuniss commented Jul 13, 2017 via email

gluck commented Oct 26, 2016 •

edited

Loading

kuniss commented Jul 11, 2017 •

edited

Loading