Add -inclusionfile command line option #2018
Conversation
Allows specifying command line options in a file to include tests, one per line. Accepted options are -[no]trait, -[no]method, -[no]class, and -[no]namespace. Accepts blank lines and comments starting with # and //.
|
|
|
The error above looks like a technical failure. I don't know how to resolve it. Can someone on the project please help? |
| { | ||
| string line; | ||
| int linenum = 0; | ||
| var spaceOrTab = " \t".ToCharArray(); |
There was a problem hiding this comment.
I'm less familiar with the files modeling in this project,
But isn't there any place for holding such static vars which will never change?
If not, at least move such thing to a static member var in this class, to be rused, rhather than generating such conversions on each iteration.
What do you think?
(I know it is minor, but maybe such accumulated improvements might bring better performance.
and asssume developper's environment is a Raspberry-Pi...)
There was a problem hiding this comment.
I considered that, but this runs during command line processing at startup; we would expect it to run once at most. Thus, it would actually be more expensive to compute this whether needed or not and save it in a static forever when no longer needed.
| string line; | ||
| int linenum = 0; | ||
| var spaceOrTab = " \t".ToCharArray(); | ||
| while ((line = input.ReadLine()?.Trim()) != null) |
There was a problem hiding this comment.
If starting to parsing files,
What do you say about using ReadLineAssync
Meaning changing the upstream flow to Async model as well, starting from the root (Program.Main)
?
There was a problem hiding this comment.
Changing the application from sync to async may be valuable, but I would consider it way out of scope for the intention of this change.
| @@ -395,6 +396,71 @@ protected XunitProject Parse(Predicate<string> fileExists) | |||
| return project; | |||
| } | |||
|
|
|||
| static void IncludeFromFile(XunitProject project, String path) | |||
| { | |||
| using (var input = File.OpenText(path)) | |||
There was a problem hiding this comment.
Consider splitting the mothos to have one method which accepts an argument of some input stream.
Then you can unit test that method using a mocked stream.
Then you chan check your varioius string parsers logic.
What do you think about that?
There was a problem hiding this comment.
It's not a bad idea at all. But considering this has languished for a year with no activity and was closed with no explanation, I don't believe it is worth my time. Apparently either this project is dead, or the people who could merge this don't consider this addition valuable, so I'm not going to waste my efforts further.
There was a problem hiding this comment.
I've understood that @bradwilson is pushing towards the next architecture of xunit V3.
V2 will not have additional builds anymore.
So if this PR is so old I would guess it is not relevant to V3.
If this is correct, I would propose to cancel this PR.
(I just thought it is part of V3 work, but was I guess I was wrong).
| var optionValue = line.Substring(spaceIndex + 1).Trim(); // If not found, this safely results in entire line, which will be ignored. | ||
| var optionName = spaceIndex < 2 || line[0] != '-' ? "invalid" : line.Substring(1, spaceIndex - 1).ToLowerInvariant(); | ||
|
|
||
| switch (optionName) |
There was a problem hiding this comment.
I think there is a lot of shared code which is copied into this method.
I suggest to move the code to a separate method and reuse them.
|
I've left this PR (and #2032, which is similar) open because I do want to support a response file. I'm not sure which strategy I would want to use yet as I'm not ready to integrate it. I may want to pull from one or both of the PRs for code & ideas. |
Allows specifying command line options in a file to ex/include tests, one per line.
Accepted options are -[no]trait, -[no]method, -[no]class, and -[no]namespace.
Accepts blank lines and comments starting with # and //.