search

nunit / teamcity-event-listener

NUnit Engine extension that helps integration with teamcity
MIT License
11 stars 10 forks source link

Handling of TeamCity messages is different between the engine and nunitlite #1

Open CharliePoole opened 8 years ago

CharliePoole commented 8 years ago

@CharliePoole commented on Thu May 26 2016

We need to examine whether the differences are OK before making changes.The main one is that there is no flowid used in nunitlite. There are other minor differences.

Once we determine what should be the same versus different, we may be able to implement some common code.

@NikolayPianikov We could use some comments from you here!

@CharliePoole commented on Tue Jun 07 2016

@NikolayPianikov After looking more closely at the differences, the biggest is that there is no flowid in nunitlite. Thinking about it, I'm still not sure why you need flowid. I understand why it was needed when NUnit failed to attach any test id to each output, but now there is no need for you to make a logical deduction about which test produced output because it is all tagged with the name and id of the test.

So maybe it's the simple approach in NUnitLite that is more correct now.

@NikolayPianikov commented on Tue Jul 19 2016

My comments about flowId: we should have real unique id's to distinguish tests in the specific period of time. The test's name (or id) is not unique for example when we run tests for similar assemblies (release/debug or different platforms .net 1.1/2/4 or something like) and we can not rely in these id's. As we discussed, flowId is required in cases when test tool produces results from different tests simultaneously. And the pair testId and flowId can play a role of unique id (as I said before, in the specific period of time only). Also we should have ability to see hierarchy of tests in this case. Bu if nunitlite does not produce results simultaneously for several tests flowId is not required.

@CharliePoole commented on Tue Jul 19 2016

NUnitLite runs a single assembly but may run with multiple threads. Each test has a separate id. Do you need flowid in that situation?

In general, whether using nunitlite or the engine, every test has a unique id. The only way duplicate ids would be produced is if you ran two entirely different copies of NUnit, which we definitely don't recommend for several reasons. NUnit expects only one copy of the engine to be active in a primary role at one time.

@NikolayPianikov commented on Wed Jul 20 2016

FlowId is needed, because we should correctly identify start and finish of a test in the case of intersection in time. FlowId could be just a test id. And parentId is not required because we have only one assembly.

@CharliePoole commented on Wed Jul 20 2016

That makes sense to me. In fact, I don't yet see why parentid is needed with multiple assemblies. If flowid is always the id of the test, then that's a unique value even across multiple assemblies.

@NikolayPianikov commented on Thu Jul 21 2016

For example:

start suite abc.dll start suite xyz.dll start test aaa in abc.dll start test xxx in xyz.dll finish test aaa in abc.dll finish test xxx in xyz.dll finish suite abc.dll finish suite xyz.dll

How we can find the relation between a test and an assembly? Should we know "in xyz.dll"?

@CharliePoole commented on Thu Jul 21 2016

The id is coded. The first part indicates the assembly.

@NikolayPianikov commented on Mon Jul 25 2016

I am not sure I understand. A listener gets the message like <start-suite id="0-1019" parentId="0-1018" name="dotNet40NUnitTests" fullname="dotNet40NUnitTests"/> for a test. The "parentId" attribute is only way to correlate this message to the "start-suite" message like <start-suite id="0-1018" parentId="" name="dotNet40NUnitTests.dll" fullname="c:\mydir\dotNet40NUnitTests.dll"/>.

@CharliePoole commented on Mon Jul 25 2016

You asked about finding the assembly, not suites in general. All ids that start "0-1" are in the same assembly. Other test fixtures and suites are a different matter. I was not aware you needed that information. Are you trying to reconstruct the entire tree from the run messages?

If you are, the most certain way is to get result of loading the assembly. However, because you are running through the console runner instead of using the engine API, that's not available to you.

So... if you need the entire test hierarchy - assembly, namespace, fixture, test cases - then you do need the parent id. If you do need it, I'd be interested to know how you use it. Getting it this way will probably miss some ignored or otherwise skipped items, so you should especially check for that.

BTW - in case you didn't know - the parentId is only present in the event messages. It's not in the final XML, where it would be redundant.

@CharliePoole commented on Mon Jul 25 2016

Unfortunately, I didn't merge this before creating the new repo - that would have been simple!

I guess at this point, the best way is for you to clone the new repo and create a branch and PR with identical code. The code for the extension right now is exactly the same in both repos.

Since you have write access to the new repo, you can simply clone it and create a branch - no need to fork unless you prefer to do it that way. In the usual way for NUnit, merging to master in the new repo will require review by one other committer.

I'll move the related issues into the new repo as well.

NikolayPianikov commented 8 years ago

@CharliePoole
Yes, I am trying to reconstruct the tree to show it in build log of TC. We produce TC messages in real time, so I should use messages for a listener. Do you think I could rely on the implicit rule "0-1"?

CharliePoole commented 8 years ago

It's not implicit at all. The definition of the id is -. This has been true for many years, even with NUnit V2. Otherwise, as you say, we could not run tests for multiple builds of the same assembly.

That said, it only gives you the assembly. To construct the entire tree on the fly, you need to have the parent id, so I think we are stuck with it.

Frankly, this is a consequence of TC's decision to use nunit3-console rather than creating it's own runner that uses the engine API. I guess somebody decided that this would save effort, but I bet you are doing more programming on handling these events than you would ever have done in creating your own custom runner, which could be getting all the information needed directly from the engine!

NikolayPianikov commented 8 years ago

Yes, I agree, but in our case an user is be able to reproduce a step of build runner locally or on an agent just from command line, without any influence from our code. Also we are trying avoid any issues with incompatibility as we discusses earlier.

CharliePoole commented 8 years ago

True. I understand the decision now in any case.

The problem, at least up to now, has een the assumption that our command-line will be more stable than the API. OTOH, we feel pretty free to make minor command-line changes, notifying users. It's less of a problem for direct users because they know what version they are using and can simply use --help to see what options are available. It's harder when the command-line is used programmatically.

What would help here is to document somewhere what command-line options you depend on. Perhaps this could be in the readme for the listener extension.

NikolayPianikov commented 8 years ago

@CharliePoole I've looked to the code and I have some points about it: