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

Add project element to top-level sub-project before merging #53

Closed
rprouse opened this issue Sep 8, 2016 · 9 comments
Closed

Add project element to top-level sub-project before merging #53

rprouse opened this issue Sep 8, 2016 · 9 comments
Assignees
Milestone

Comments

@rprouse
Copy link
Member

rprouse commented Sep 8, 2016

This is a remaining issue from PR #52 which is intended to fix #30. As @CharliePoole stated in that PR,

This looks much cleaner! The remaining problem I see has to do with where MakePackageResult is called. You moved it from the individual runners to MasterTestRunner. I like the way that removes duplication, but I'm not sure it works correctly now.

Here is what I think will happen, depending on what's on the command-line:

Arguments Outcome
One assembly No project element, as expected
Multiple assemblies No project element, as expected
One project One project element, as expected
Multiple projects No project elements rather than one per project
Project plus assemblies No project elements rather than one wrapping just the project assemblies

The problem is that the project element, if any, needs to be added to each top-level sub-project before they are all merged.

That said, this is a rather esoteric use case. I'd be OK with merging this and creating a new issue for the edge cases if @rprouse agrees.

@CharliePoole is there more information you would like to add to this issue?

@CharliePoole
Copy link
Collaborator

Looks good. I'll take this one and also add comments for some of the esoteric bits that are liable to confuse.

@CharliePoole
Copy link
Collaborator

I've just been bitten by this one in TestCentric. When opening a single project, containing multiple assemblies, there is no top-level project element present. This is actuallly the "One Project" place I commented on above, which I thought was working.

See TestCentric/testcentric-gui#159

@ChrisMaddock
Copy link
Member

@CharliePoole could you help my understand what the issue is here, in terms of TestPackage structure? i.e. What we currently have, and what would be desirable? I'm assuming when we talk about "project element's" here, that relates directly to test package structure? Or is this a separate issue?

@CharliePoole
Copy link
Collaborator

By "project" I mean anything the engine recognizes as a project through one of the extensions, like NUnit projects or Visual Studio projects and solutions.

Projects cannot currently be nested, so they can only appear at the top level... command-line or constructed in the GUI by adding more files to those that are open.

Whenever there is a project, there was supposed to be a <test-suite type='Project'/> node in the tree.

There is code to create a project node in the engine, but I'm not seeing any being created. I haven't explored all combinations of projects and assemblies to be sure. Back when Rob commented, it seems like we were only creating a project node when there was a single project, by itself on the command-line. I know we are not doing so now when I open a solution file.

The test package structure derives from what is entered on the command-line or may be created in other ways in a GUI. Because of that tie-in, I was able to deduce what the engine should have produced in the GUI issue and patch the test tree to have the necessary nodes. That's a real hack, of course.

@ChrisMaddock
Copy link
Member

@CharliePoole So just to confirm - the TestPackage structure here is currently correct, but there is a failure in the xml produced?

I ask, as I'm currently looking at TestPackage structure, but not 100% clear on where the TestPackage structure is a) incorrect itself, or b) incorrectly handled.

@CharliePoole
Copy link
Collaborator

The TestPackage is actually created by the runner. The engine fills in subprojects. I haven't seen anything wrong so far in how the engine does that. In fact, in my GUI issue, I'm relying on the TestPackage structure to provide the extra level of info that is missing from the XML that is returned from the engine. IIRC, we have tests that check the package structure for various combos of assemblies and projects.

The trick is for the engine to use the package structure so as to create a project node when it aggregates the assemblies under a project. That's what it isn't doing. When I run my GUI solution file, I get back a test-run element with 11 child test-suite elements of type Assembly.I should bet back a project node in between the test run and it's children.

@CharliePoole
Copy link
Collaborator

To be clear... every runner has a test package that it is running it may be just an assembly or a collection of assemblies. If the test package is recognized as a project, then a project node should be created on top of the returned assembly or assemblies. We need tests for this but they would be fairly high level, i.e. loading one or more assemblies.

@CharliePoole
Copy link
Collaborator

This continues to be an issue for TestCentric GUI. As far as I can tell, there are no circumstances in which project elements are produced.

@CharliePoole
Copy link
Collaborator

CharliePoole commented Mar 19, 2019

Currently, the only code in the engine that creates a project element is the oddly named MasterTestRunner.PrepareResult. It creates a project element IFF the top level element is a project, which in fact it never is, rather the top level element is the anonymous command-line package, which may contain one or more projects.

In principle,depending on settings, any of the TestEngineRunner results may be for a project or part of a project. So, in theory, this could be handled at a lower level or at the master runner level using more complex code.

The original change, cited at the top of this issue, was working at first for cases with a single project, by itself. That's because we used to handle the case of a top-level single project specially, replacing the anonymous command-line package. That created some pretty obscure code and at some point it got removed, breaking what is essentially an untested feature.

IMO, we need some true, separate integration tests that actually run the engine and examine the results for all these cases. I think this belongs in a separate project, so I'm not going to try to create them now for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants