-
Notifications
You must be signed in to change notification settings - Fork 152
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.
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
Allow users to pass arguments via file(s) instead parameters of a command line #94
Comments
In the past, I was accustomed to the syntax "@filename" to represent a file whose contents were interpolated into the command-line. Is that too archaic a convention to use today? It would be a convenient way to pre-process args before handing them to Mono.Options to parse. I'm for this as a feature if we can agree on a syntax. I'm up for repeating it on the command-line but I don't like the idea of nesting. |
I like the idea I agree that the nesting is not useful in general scenarios, but for this argument we should check a source of the argument each time and we should raise an error if the argument was defined in a file. Also I spent some time to see how settings are working now and to write tests for settings before making changes. I found that we have no any unit/integration tests for it. I started from prototyping and found that we have no way to write "real" unit tests - we are not using any frameworks for mocks so far. It is possible to choose any and use it everywhere? |
If we treat it as a pre-processing step |
If we treat it as a pre-processing step, then the options code won't even understand the format and should give an error. We have unit tests for all our settings. We don't have real integration tests for anything. Maybe you mean something else? |
We use nsubstitute |
Ok I will use I mean unit tests for What do you think: should we use a file's format like for the command line - the single line in a file? Or we parse the each argument from the own line? I going to make a simple implementation to have criticism from your side. |
Is the order of the arguments essential? |
Options.cs is imported from mono, where it is tested. In principle, we don't modify it but we made a few small exceptions to make it build on certain platforms. I'm hoping to back those changes out after we remove CF from the framework project. FYI, I also have plans for a major revision of how we do options, eliminating the use of Mono.Options, which has some limitations for our purpose and yet is also more complex than we really need! We should only modify and test ConsoleOptions. Suggestion: Create a static Expand method in ConsoleOptions and change the constructor to call |
When I implemented this in my CF build (nunitlite), I used the The reason I never backported the implementation to the full nunit framework was that we never were able to agree upon a number of issues, and also that I was unfamiliar with the nunit-console, so I needed someone to port the nunitlite implementation to it if we went that way. I implemented two level nesting, but @CharliePoole did not want nesting. I allowed both a single line and a multi line format, with a number of options on the My implementation is a preprocessor one, like the implementation of the --testlist option. Nesting allows there to be files containing fixed sets of options that are used in every test suite to be included with test suite specific options easily. My implementation and the tests for it has been in use for over a year in my Crestron CF build. |
Simplicity (for users) is something we always strive for in NUnit. When given a choice between implementing what may be too little or may be too much, we choose to risk too little because features can always be added but - once added - can only be removed with difficulty. Other projects, of course, have different values and act differently. One thing to be sure of in any of these discussions: when we are talking about complexity, we are not saying it is too hard to implement. We are referring to the complexity that gets presented to the user. Since we are programmers and so are our users, it's tempting to try to wear both hats. I'm happy to say that I got over that habit a few years ago. Let's implement the "simplest thing that can possibly work" and wait to see what the users ask for once they see that. |
Usefulness is important as well. I found that having Having only a single line in the file, or having only one option per line, both limited the usefulness of the Simplicity is not always the same as ease of use. |
Simplicity and ease of use are somewhat moot, if no feature ever gets implemented. @NikolayPianikov 's proposal is 14 hours old. We've discussed it and settled on some features, which he is working on. That seems like good progress to me. |
As I said, I implemented this in my nunitlite CF build over a year ago, but you were not interested in my implementation. Backporting to full nunitlite would take about an hour. It would, however, not do anything for nunit-console. |
@oznetmaster Although I didn't know you had implemented it privately, I'm not surprised. There has never been any doubt in my mind about your ability to implement whatever you choose to implement. However, your implementation never arrived at our users, which was my point. Saying that was because I was "not interested" is unfortunate because it's neither true nor useful. I'm interested in getting features to users. @NikolayPianikov made a basic proposal. I made a counter-proposal as to format, which he indicated he liked. We asked each other questions. We compromised on an approach in a matter of hours. We'll do more of that when there is a preliminary bit of code to work with. Our users will get some minimal feature set to try and then we'll look at it again in a few months and possibly give them more. This way we will minimize the risk of taking features back that prove unworkable after use. This is what happens with most of our features. If we need to discuss the past proposal any further, let's do it by email. @NikolayPianikov created this issue to discuss his proposal, not past history. @NikolayPianikov If @oznetmaster has code to parse multiple options out of a line in the file that he is willing to contribute, you might consider using it rather than re-inventing. |
@oznetmaster @CharliePoole It would be great to use ready-made solutions, rather than inventing their new bikes. @oznetmaster could you point me which classes I could use or could you append code to parse files to the nunit3-console project (with tests if it possible) or if you prefer to implement this feature in full let me know. |
It is interesting that my code for this in CommandLineOptions (written a year ago) looks virtually identical to the code written by @NikolayPianikov . The only difference is that mine supports recursion, and mine supports both option per line (with delimiter override) and single command line formats at the same time. About 10 additional lines of code. I guess there is really only one way to do this easily :) |
… of a command line - fix according to review, support the lazy expansion
@oznetmaster Maybe because we are thinking the same way? :) |
… of a command line - implementation allowing to parse argument with spaces and multiple elements per line
@rprouse @NikolayPianikov Is there any reason this should still be open? |
Nope, the PR was just missing the |
It's important because arguments containing a bunch of assemblies with full paths have a significant length. And users should split these assemblies to run tests in several sessions or use .nunit configuration files. It could be implemented like:
--arguments=filename
and-a=filename
File name contains the text similar a command line.
For example a command line could be:
nunit3-console.exe -a=myArgs
where the
myArgs
file contains one line:assembly1 "assembly 2" assembly3 --noheader --teamcity --x86
or several lines:
assembly1 "assembly 2"
assembly3
--noheader --teamcity --x86
Also in the simplest implementation we can use one command line argument per line, because we could avoid the question how to deal with symbol ".
Theoretically:
-a
argument could be used several times in the command line.-a
The text was updated successfully, but these errors were encountered: