Skip to content
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

[WIP] Add project result elements to XML #582

Closed
wants to merge 1 commit into from
Closed

Conversation

CharliePoole
Copy link
Collaborator

@CharliePoole CharliePoole commented Mar 21, 2019

This will fix #53 but is currently partial only.

One thing leads to another! Currently, everything seems to be working correctly except for when you use `--process:Separate. When that happens with one or more projects in the command-line, the whole job of figuring out the proper hierarchy falls to the agent process. This is more work than the Process Runner and agent process were originally designed to handle so changes will be needed.

I see some choices for this issue...

  1. Merge what's done and leave the --process:Separate case for fixing later. (3 PRs)
  2. Put this on hold until the problem with --process:Separate can be resolved. (2 PRs)
  3. Make resolving the --process:Separate part of this PR. (1 PR)

As anyone who knows me would guess, I prefer the 3 PR approach. I'd make them all steps in the same issue, however, since the problem really only comes up when trying to add XML elements for each project. Kind of depends on how comfortable folks are with merging code without knowing what the next step will be...

@CharliePoole
Copy link
Collaborator Author

There is an alternative. The TestCentricGUI currently examines the XML returned by the engine and re-interprets it in light of the structure of nested test packages it is attempting to run. It injects XML elements so that the test hierarchy that is displayed in the GUI contains nodes for each of the projects.

The downside of the GUI doing this is twofold

  • It only works for the GUI
  • The nodes are fake. The engine knows nothing about them so any action that would normally invoke the engine passing one of these nodes has to be emulated in the GUI.

I'm wondering if the same code could be ported into the engine MasterTestRunner, eliminating both downsides and keeping the logic on one place, rather than distributed in each type of runner.

I'm going to create an alternate PR for this issue.

@CharliePoole CharliePoole changed the title Add project result elements to XML [WIP] Add project result elements to XML Mar 21, 2019
Copy link
Member

@rprouse rprouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with a three PR solution, but let's wait until you experiment with your other PR. Other than that, this looks good.

@@ -13,7 +13,6 @@
<ItemGroup>
<PackageReference Include="NSubstitute" Version="2.0.3" />
<PackageReference Include="NUnit" Version="3.11.0" />
<PackageReference Include="NUnit.Analyzers" Version="0.1.0-dev-00079" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the Analyzers causing problems for you? I like the idea of dogfooding the project, but if it is getting in the way...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good illustration here. Dropping the analyzers got checked in by mistake. I generally drop them since I can't build in my admittedly strange environment, but avoid committing that change. It's another step to remember to take.

We dogfooded a lot at Microsoft, but it always meant using production tools. For NUnit, we test the framework using the framework under development but using a released console or adapter. We test the adapter using a released framework, etc. That way we are testing only one thing at a time. The analyzer is the only exception to that I know of. OTOH, I'd really like to know why it causes a versioning conflict in my configuration that @mikkelbu can't duplicate.

@CharliePoole
Copy link
Collaborator Author

Alternate fix was merged. Deleting this PR.

@CharliePoole CharliePoole deleted the issue-53 branch April 6, 2019 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add project element to top-level sub-project before merging
4 participants