Presence of a TEAMCITY_PROJECT_NAME environment variable makes builds fail if TeamCity service unavailable

[stinos]

With release 3.19.0 in, this commit leads to TeamCity CI builds failing for us with Option --teamcity specified but the extension is not installed. even though we do not specify this option and we do not use any of TeamCity's nunit-related features: we just happen to use nunit3-console to run tests.

• The full command line being used: nunit3-console --noheader --noresult /path/to/tests.dll
• Method of installation (e.g. NuGet, zip, msi): NuGet, but not relevant
• Version of the NUnit Engine/Console: 3.19.0

Can be reproduced easily by setting a non-empty TEAMCITY_PROJECT_NAME environment variable.

So:

• the error message says --teamcity is specified even though that is not the case, which is a bit confusing
• the Options code treats the presence of a TEAMCITY_PROJECT_NAME environment variable as running under TeamCity. While correct, it also treats that as 'must use TeamCity extension', but that is for us not the case

I'd suggest that either the behavior is reverted, or modified to show a warning instead of an error (perhaps with a modified message stating 'TeamCity detected' or similar), or else made such that the user needs to explicitly request the TeamCity functionality (so by passing --teamcity or by another environment variable).

[CharliePoole]

To explain why it works this way, I'll have to go back in time, to 2015. :-)

That's when @NikolayPianikov, a contributor for JetBrains, added the notion of a DefaultOptionsProvider class, implementing IDefaultOptionsProvider. This class does what it says. It specifies default option values for the console runner, over-riding any internal defaults.

The class was used, in this case, to make the TeamCity option appear to be true, just as if you had entered the --teamcity option. JetBrains wanted this so that users would not need to specify the option on the command-line.

We'd like to keep this working for JetBrains users, so that limits what can be done for this issue without a bit of coordination. Before we go further, can you explain your use case? Why do you want to run under TeamCity, but not use the TeamCity extension. I have limited contact with the JetBrains folks, but my understanding from back then was that their display of results would be broken if the teamcity extension were not intalled.

Depending on how this discussion turns out, I'll either make a simple change to the wording of the message or I'll queue up a task to revise how we handle options in general. Currently, the runner simply gets settings, without knowing whether they came from the command-line or elsewhere.

[stinos]

We'd like to keep this working for JetBrains users, so that limits what can be done for this issue without a bit of coordination.

Sure, I kind of expected that.

Before we go further, can you explain your use case? Why do you want to run under TeamCity, but not use the TeamCity extension

We have a project which is a mix of C++/C#/Matlab/Python/Installshield/.. and want our build/test/deploy to be a one-click kind of thing which can easily run anywhere. As such there's a single msbuild target which does everything, and that happens to include running nunit3-console just like it runs the other non-.Net tests via commandline commands. So CI builds are no different and also run that single msbuild command and we don't use any test integrations. Makes things easier to setup and maintain and avoids some of the 'does(n't) work on CI but does(n't) work locally' type of issues.

[CharliePoole]

I'll change the message to be clearer, since you don't use the --teamcity option.

If you don't use it that option, your code should run locally or under TeamCity but requires the extension otherwise. Isn't that what you would want? Does your CI server have a TeamCity installation without the extension? I'm not a TeamCity user, but the folks there tell me the extension is part of their normal install process.

[stinos]

Does your CI server have a TeamCity installation without the extension?

Not sure, our TeamCity installation doesn't even explicitly haven NUnit Console installed, but I think that is not relevant for our usecase: we install the NUnit.ConsoleRunner package via NuGet as part of the build script and then use that. Build-wise all we use TeamCity for is calling msbuild main-build-script.proj.

FWIW workaround used now is along the lines of (set TEAMCITY_PROJECT_NAME=) & nunit3-console /path/to/test.dll i.e. erase the environment variable. Though I now see there's also an NUnit.Console package on NuGet which does include the TeamCity extension so that might actually work as well..

[CharliePoole]

I'll talk to someone from JetBrains about a better way to handle this.

You could also install NUnit.ConsoleRunner plus NUnit.Extension.TeamcityEventListener if you don't use any other extensions.

[BlythMeister]

I have hit this same issue. We don't use the TeamCity listener (even though we run on TeamCity). Before this version we had no issues, but now we get this error message. Is there scope to add a "--no-teamcity" arg so when running you can force nunit to ignore that environment variable :D

[CharliePoole]

OK, we recognize the environment variable as equivalent to "--team-city" because JetBrains requested it and we were given to understand that anyone running under TeamCity had to use the listener. It's clear this doesn't work for all users.

I'm not keen to add yet another vendor-specific option to the runner like "--no-teamcity". Therefore, I'll investigate the following option first...

We have a built-in capability to enable and disable extensions. The TeamCity extension is disabled unless you are running under teamcity (indicated by the environment variable) or use the "--teamcity" option to enable it. This capability is not exposed in the runner in a more general way but was planned as a new version 4 feature. It looks doable without too much effort, so I could advance it to 3.20.

In your situation, you would use "--disable:NUnit.Engine.Listeners.TeamCityEventListener" or possibly the shorter "--disable:TeamCityEventListener." The latter may be postponed to V4, if it takes too much effort on my part. I'll add an "--enable" option as well and I'll deprecate the existing "--teamcity" option.

NOTE: If you enter conflicting options, the last one encountered "wins". The recognition of the environment variable takes place before any options are parsed, so teamcity will be enabled and then disabled if you use the "--disable" option. Individual extensions specify their initial state using an attribute, but they don't execute any code until called by the engine.

If something turns up to block the above, I'll do the "--no-teamcity" option and deprecate it immediately.

@nunit/engine-team @stinos @BlythMeister @NikolayPianikov I'd appreciate your comments on this approach.

[stinos]

"--disable:NUnit.Engine.Listeners.TeamCityEventListener" or possibly "--disable:TeamCityEventListener"

Any such commandline option would be fine for me, and I don't think this would cause unforseen issues.

Thanks for looking into this.

[BlythMeister]

As an idea.

The new error message makes sense IF you have included --teamcity.
But, does it make sense if it's detected TeamCity?

Would it not be easier to only throw that new error if the TeamCity arg is passed?

Having the disabled option works fine for me, but thinking about the discover ability.

For context, the reason we dont use the listener. Our build script uses cake or fake (depending on version) ans takes the unit XML and passes the whole lot to TeamCity. We have a lot of tests and means we can do some pre-processing on the XML before letting it hit TeamCity.

[CharliePoole]

@BlythMeister I agree. In the current code, it would require a bit of restructuring in order to recognize that the "option" was not actually entered at the command-line. The person who implemented it went to a bit of trouble to make it appear as if the option had actually been entered.

Not giving any error, even if we recognize the difference, would confuse a lot of people. In fact, that's why we added the error message: people assumed that "--teamcity" just made things work and knew nothing about the extension. However, I can make the option message say something like "you either used the option or are running under TeamCity". Unfortunately, we are dealing with some history here and it's hard to make a change that won't disrupt at least some users. With V4, of course, we can be as disruptive as we like. :-)

[CharliePoole]

It would have been better, in fact, to have the environment variable notify the console that it is running under teamcity, which is what it really means rather than "user wants teamcity event processing." Let me consider that as an interim change.

[BlythMeister]

Yeah that's what I was trying to get to, but didn't convey it as elequantly 🤣