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

How to skip non-test .NETStandard assemblies? #900

Closed
synek317 opened this issue Feb 20, 2021 · 9 comments
Closed

How to skip non-test .NETStandard assemblies? #900

synek317 opened this issue Feb 20, 2021 · 9 comments
Assignees
Milestone

Comments

@synek317
Copy link

I have a solution with multiple projects. My current environment is Linux + Mono + nunit3-console. I used to use nunit3-console ... --skipnontestassemblies *.sln.

Now, I'm trying to make my project compile on both, mono and .net core. So I'm porting my libraries to use framework netstandard20 while keeping test assemblies to use net461.

Unfortunately, nunit3-console no longer skips my non-test assemblies because it discovers unsupported framework before it checks if this assembly has tests or not. What is even worse, I cannot try with <TargetFrameworks> instead of <TargetFramework> because of a bug in the project loader: nunit/vs-project-loader#37

Is there any other way I can keep passing sln path to nunit3-console and skip unsupported assemblies?

@ChrisMaddock
Copy link
Member

ChrisMaddock commented Feb 27, 2021

HI @synek317,

The VS Project Loader hasn't had a maintainer for the last few years, and hasn't really been updated for some of the later features of csproj files. @CharliePoole has recently become maintainer of this project and is currently considering it's future - I'm sure he'd be interested in any offers of help!

Can you clarify - do your netstandard20 assemblies contain the [NonTestAssemblyAttribute]? And you're saying that the engine errors before it checks for the attribute? That does sound like a bug - would appreciate a pull request to fix it, if you're interested? 🙂

@CharliePoole
Copy link
Collaborator

@ChrisMaddock Did you want to hang onto this one for a bit rather than transfer to vs-project-loader? That's OK with me, if so.

@ChrisMaddock
Copy link
Member

I think this issue is in the correct place, as reported, this is an engine problem. There are a couple of other considerations for the extension beyond this perhaps however, here's what I think:

For the engine:

  1. The Engine should probably give precedence to NonTestAssemblyAttribute ahead of throwing for assemblies that target a platform it doesn't recognise. (As filed in this issue!)

For the Extension:
2. nunit/vs-project-loader#37, as @synek317 has already filed. That's dependent on whether the extension is updated to fully support the new csproj or not. (And a question for the maintainer, in my eyes. 😉)
3. What, if anything, should the VS Project Loader extension do by default around EnginePackageSetting.SkipNonTestAssemblies? Here it's being added on the command line. But I'm almost tempted to say the VS Project loader should set that by default when the user is using a sln file? Purely on the basis that I think it's quite unusual to have a solution containing only test assemblies - far more normal would be both production code and test assemblies in the same sln. That change would also depend on #827, perhaps.

@CharliePoole
Copy link
Collaborator

Ah that makes sense! I believe that the package created by the VS Project Loader extension for a solution should set SkipNonTestAssemblies because not doing so generates errors. This needs to be done no matter how #827 ends up interpreting the setting - although we should discuss that in #827.

@synek317
Copy link
Author

Thanks for mentioning [NonTestAssemblyAttribute]. It might sound surprising to you, but I haven't known it and my experience was closer to what #827 describes. Tbh, I was pretty sure that nunit-console considers every assembly without the reference to NUnit package as a "non-test".

I will check how adding this attribute works in netstandard + net4x (+net5 maybe) mixed solution.

@CharliePoole
Copy link
Collaborator

There's a bit of a problem in that [NonTestAssembly] may have evolved away from it's original intent. However, since I wrote it, here's what that intent was:

  1. Having a reference to a known framework (e.g. nunit) normally makes a test a "test assembly".
  2. Using [NonTestAssembly] on such an assembly creates an exception to the first rule.

IOW, you should only need the attribute on an assembly, which is not a test assembly but has a reference to the framework. An example would be an assembly containing your own extensions to the framework.

Knowing the definition, however, is only useful in deciding whether it's your code or the engine's that needs to be fixed. 😄

@CharliePoole
Copy link
Collaborator

@nunit/engine-team I hadn't really thought about this before, but maybe part of the problem here is that the definition of NonTestAssemblyAttribute is different from what is implied by the console runner option --skipnontestassemblies and the corresponding package setting SkipNonTestAssemblies.

Cases we may need to deal with...

  1. A normal test assembly, with tests
  2. A normal test assembly (i.e. with a reference to the framework) but without tests having been added.
  3. A utility module with a reference to the framework, which should never be treated as a test assembly at all.
  4. A non-test assembly in the sense that it neither has tests nor references the framework.
  5. A test assembly for some framework we don't know about.

The attribute is only useful for case 3, since you can't have the attribute without adding a reference to the framework. Without the attribute, cases 2 and 3 are indistinguishable.

But when a user uses the console option, I think they are really talking about case 4.

What do you think? How should we deal with this?

One possibility: always skip tests with the attribute and additionally skip tests without a reference to a framework if the option is used. But what do we do about case 2, where there's a reference but no tests. Is that common enough to be important?

@CharliePoole CharliePoole removed their assignment Jan 1, 2023
@CharliePoole CharliePoole self-assigned this Sep 19, 2024
@CharliePoole
Copy link
Collaborator

CharliePoole commented Sep 19, 2024

@synek317 I realize this is pretty old. Assuming you are still around, can you try the latest dev build from our myget feed? I've made substantial changes in the engine in order to compensate for unsupported frameworks and unmanaged assemblies throwing an exception when we try to analyze them. These changes will be in 3.18.2 and I'd like to report this as fixed but I don't have a specific repro to work with.

UPDATE: I went back up and read the title. I do have a test using a solution with a .NETStandard assembly, so I'm going to report this as fixed. Feel free to tell me if you still find a problem and I'll re-open.

@CharliePoole
Copy link
Collaborator

This issue has been resolved in version 3.18.2

The release is available on:
GitHub.
NuGet packages are also available NuGet.org and
Chocolatey Packages may be found at Chocolatey.org

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

No branches or pull requests

3 participants