nunit / nunitv2

NUnit 2.x repository. NUnit V2 IS NO LONGER MAINTAINED OR UPDATED. Bugs should be reported in the main NUnit 3.0 repository.
https://github.com/nunit/nunit
Other
25 stars 26 forks source link

Migration version #43

Open NN--- opened 7 years ago

NN--- commented 7 years ago

Please release a new minor updated version of NUnit 2 with marking methods, which are removed in v3, obsolete. This would make easier transition to v3 from large code base.

A possible workaround for Resharper users is to add external annotations: http://hmemcpy.com/2013/03/even-more-resharper-annotations-hackery-marking-3rd-party-code-as-obsolete/

CharliePoole commented 7 years ago

We had previously said we would not release any more versions - unless something came up. 😄

If somebody 😄 were willing to do the work, it's possible we would consider it.

I don't know what your skills are or if you have the interest, But if we leave this here there's some small chance that somebody will come along. Otherwise, the time of the existing team is pretty well taken up with NUnit 3.

@rprouse I'd like your thoughts on this. I'm willing to see further minor releases of V2 if somebody other than the existing team wants to commit to it. Would you buy into that?

NN--- commented 7 years ago

It is just adding Obsolete attribute for v2 users for easier migration.

Currently I see many usages of removed features. I would like to see warning on them to switch to v3 when it will be appropriate without changing code.

Thanks.

CharliePoole commented 7 years ago

That's understood. However, there is a certain amount of work involved in doing even the simplest release, so unless other @nunit/core-team members think differently, we won't be updating this project.

You may not have realized that my suggestion was a very large concession to your request. Up to now we have refused to update nunit V2 even if other people provided the updates. In fact, we even merged one or two small PRs to the source, for those who build from source.

Let's wait to see what the others think.

rprouse commented 7 years ago

My feeling is that we should only release a new 2.x release to fix major bugs or to do something that would encourage people to move to NUnit 3 which this might fall under.

That said, there are very few removed features other than the ExpectedException attribute. There are changes and additions, but that is the only pain point that most people mention. The other is switching the setup attributes, but that is a global find/replace to upgrade.

Adding the Obsolete attribute would introduce many warnings into code the is perfectly acceptable. This means that anyone with warnings as errors turned on would be forced to fix the code if they took the new version. To me, that is a breaking change.

Personally, I think the easiest migration path to NUnit 3 is to just do it and fix the compile errors. For most users, it is fairly quick and painless except for ExpectedException.

What other missing features are you seeing?

NN--- commented 7 years ago

There are plenty removed methods: https://github.com/nunit/docs/wiki/Breaking-Changes#assertions-and-constraints

BADF00D commented 6 years ago

I would do the work. But due to the comment from @rprouse, I'm not sure whether the resulting pull-request is going to be merged. I don't want to wast my time, so I need an answer in advance.

I know that there are some people that treat warnings as errors. but nobody forces them to update. And others that want to migrate step-by-step need some warnings while editing their code. Our company has roughly about 80000-100000 tests. So we have no choice then migration step-by-step.

My only other solution is, to create a Visual Studio Extension, that uses Roslyn to find all deprecated constructs.

rprouse commented 6 years ago

@BADF00D Other than the ExpectedException, most of the deprecated methods are still in all currently released versions of NUnit 3. The problem with a staged approach using 2.6.4 is that some of the methods that you would migrate to are not present. By updating to NUnit 3, you get the deprecation warning that you are asking for. You can add in a temporary ExpectedException as a crutch (see the C# samples project) and then the few compile errors for the missing methods will probably be a global search and replace.

Maybe I am over-simplifying, but have you tried this with one of your test projects? We only have thousands of tests, but the upgrade went very quickly for me.

CharliePoole commented 6 years ago

@rprouse One of the things I have discussed doing separately from the NUnit team is providing legacy support. This issue night fit with that although I have some of the same reservations you do. I'd want to hear more details of what @BADF00D is proposing first.

dscopra commented 6 years ago

Actually we upgraded three small repositories last week with say about 5000-8000 Test. This was a test case to check, what have to be done to upgrade when we update our main repository. Within this test, I stumbled upon the following problems.

  1. Missing ExpectedExceptionAttribute (one time with pattern matching)
  2. Missing Throws method
  3. IEnumerable have to be static
  4. async void does not work any longer (I know this is is not recommended at all, but unfortunately it worked before and so there were some test using this feature)

It was no problem at all to fix point 3 and 4. You can simply do a search and replace, as @rprouse already mentioned. Unfortunately we use the ExpectedExceptionAttribute and Throws very often. All in all we needed about 20 hours to migrate the three repositories.

My conclusion is, that we can't migrate our main repository in one step. That would simply cost to much time at once. But we will have a meeting to discuss this, today.

For the future, I have two goals:

  1. As long as we do not upgrade, I don't want that any developer enlarges the problem by adding more test using the deprecated features.
  2. I want to give the developers a hint, that the legacy test they are currently working on, might get a problem in the future in order to encourage them to fix this.

At the moment I can see three possible solutions.

  1. Put an ObsoleteAttribute on ExpectedExceptionAttribute and Throws method and release this as new version.
  2. Write an Extension for Visual Studio to do the work. I wrote a proof of concept on Friday and it worked fine.
  3. A combination of both.

The second solution tackles all mentions problems, whereas the first only tackles the some of them. For the record, I'm fine with all solutions, because our problems would be fixed. But I think the third solution is the better option for the majority of people.

Screenshot from the proof of concept Visual Studio Extension:

image

ChrisMaddock commented 6 years ago

As a way of easing the switch, have you considered adding custom attributes/constraints for ExpectedException/throws constraint? See the nunit-c#-samples repo for an example of that.

ChrisMaddock commented 6 years ago

I'd personally much prefer to see a Roslyn analyser with built in code fixes to tackle this problem, over another 2.x release - however I understand your issue, and how it would help.

ChrisMaddock commented 6 years ago

Have you tried a variety of assemblies? Doing ours - I always found the changes you've mentioned the easiest to solve - the problematic ones were filepaths/changes to make testcasesources static - which can't really be tackled by a new Nunit 2 release. (Sorry for the multiple posts - my app doesn't like newlines today it seems...)

rprouse commented 6 years ago

@dscopra thanks for that update, it is an excellent overview of the pain of upgrading. As I've always suspected, the worst problems are around ExpectedException and its related syntax like the Throws() method. As @ChrisMaddock says, you can get part way there by adding ExpectedException back into your code with https://github.com/nunit/nunit-csharp-samples/blob/master/ExpectedExceptionExample/ExpectedExceptionAttribute.cs. I would mark your version as deprecated though 😄

I'm not sure about the other usages like Throws(), I would need to look at the code to see if you could derive from TestCaseData for example.

CharliePoole commented 6 years ago

I'm interested in this as a potential project, external to NUnit itself.

As the @nunit/core-team members know, I'm evaluating options for things to work on in my post-nunit-project software life. 😄 One of the things I mentioned in our discussions was legacy versions of the software. I'd fork the nunitv2 project and do a bug fix release on two timeframes: on-demand for paying clients and annually or semi-annually as open source. In this way, the paying clients would actually be supporting the open source releases.

My plan was to fix bugs only, but I could see providing enhancements that help and/or encourage conversion to V3.

I wasn't quite ready to do this, but this issue has pushed it to the head of the queue. Before I take any more steps, I'd like to hear what @rprouse and the @nunit/core-team think about it, as well as what the proposers of this issue and other users think.

BADF00D commented 6 years ago

Hi, I just released a first version of the extension for Visual Studio 2015 and 2017. Fell free to test and criticize it. SwitchToNUnit3 in Visual Studio Gallery