tintoy / corefx

This repo contains the .NET Core foundational libraries, called CoreFX. It includes classes for collections, file systems, console, XML, async and many others.
https://docs.microsoft.com/dotnet/
MIT License
1 stars 1 forks source link

Add mono tests #6

Open anthonylangsworth opened 7 years ago

anthonylangsworth commented 7 years ago

Mono already includes tests for the System.Security.Cryptography.Xml namespace. Convert these from NUnit to Xunit and get them compiling under .Net Core.

The plan:

  1. [x] For each file:
    1. Change the existing NUnit using statements and Xunit.
    2. Convert the NUnit Assert method calls to Xunit equivalents. Remove messages if no equivalent Xunit version exists. Replace [ExpectedException()] with Assert.Throws<>.
    3. Remove [Category] attributes, particularly for "Not Working" style categories.
    4. Change the comment at the top of the file as per @danmosemsft 's post below.
    5. Change the namespace from MonoTests.System.Security.Cryptography.Xml to System.Security.Cryptography.Xml.Tests.
    6. Reformat the file as per .Net Core standards. (Import corefx.vssettings then run "Edit" -> "Advanced" -> "Format Document" or Ctrl+E, D.)
  2. [ ] Fix or remove failing tests.
  3. [ ] Clean up, including:
    1. Rename files from Test,cs to Tests.cs.
    2. Where possible, use new Xunit features, such as Assert.Equals() accepting IEnumerable<T> parameters.

Note that, to prevent the branches from growing stale and to encourage review, we are merging this now and continuing to work on it.

danmoseley commented 7 years ago

Here's the magic to do to the copyright headers when we pull code from Mono:

1. Keep the existing copyright headers in place
    ○ Note: If you are porting just portions of the file over, you still need to bring over all the existing copyright headers.
2. Add the following copyright headers – NOTE this is missing the middle line of the ‘regular’ CoreFx copyright headers:
             // Licensed to the .NET Foundation under one or more agreements.
             // See the LICENSE file in the project root for more information.
anthonylangsworth commented 7 years ago

Status update: 107 failing tests remaining. I will work through them this week.

CC: @tintoy

tintoy commented 7 years ago

@anthonylangsworth anything I can do to help?

peterwurzinger commented 7 years ago

Have you got a guess on how high the coverage percentage will be when all Mono-tests are up and running @anthonylangsworth ? For me it seems that this is more than just a fair start of unit testing, maybe it's already enough to go on to the next phase..?

anthonylangsworth commented 7 years ago

I was wondering something similar, @peterwurzinger. I gave myself five minutes but could not get it working. I'll try again once I get the tests working (hopefully this week but I said that last week).

Jaedson33 commented 7 years ago

Are there any failing tests?

anthonylangsworth commented 7 years ago

@Jaedson33 At the time I am typing this, there are still 84 failing tests in the tests copied from Mono. However, many are due to .Net core limitations and changed defaults. No bugs so far.

Once we have all the Mono tests compiling, we'll still likely need to expand the list of tests to ensure the spec is covered. We have no articulated quality requirement but the .Net core team will likely be fussy.

I realize this may be frustrating for you. If you want to help, have a look at https://github.com/tintoy/corefx/pull/7 then pick a class or classes with failing tests. Let me and @tintoy know so we do not duplicate effort.

Otherwise, you could always take the code from this branch and try it out. We can write tests to cover any issues you find and try to get them fixed promptly.

anthonylangsworth commented 7 years ago

Down to 49 failing tests.

Jaedson33 commented 7 years ago

Why I'm having this error? ResolveMatchingContract could not find a matching contract 'C:\Users\Jaedson\Source\Repos\corefx\bin/ref///System.Security.Cryptography.Xml.dll' not found. System.Security.Cryptography.Xml

tintoy commented 7 years ago

Can you tell me a bit about your system? What operating system (e.g. Windows 64-bit, or OSX 10.11.6), and what command are you running when you see the error?

tintoy commented 7 years ago

(I assume it's Windows from the path but it's always worth checking)

Jaedson33 commented 7 years ago

My OS is Windows 10 64-bit and I'm trying to compile the solution.

tintoy commented 7 years ago

In Visual Studio? Or from the command line.

You have to run some command-line stuff the first time you pull down the code before you can do anything else.

Jaedson33 commented 7 years ago

In Visual Studio... What should I do?

tintoy commented 7 years ago

Full instructions here, but you'll probably be fine if you just:

  1. Open a command prompt (there's a start menu item for it, from memory)
  2. Go to the root of the repository
  3. Run build.cmd.
tintoy commented 7 years ago

Which branch are you working on?

Jaedson33 commented 7 years ago

I'm trying to work on monotests.

tintoy commented 7 years ago

That should be all you need then.

BTW first-time build may take a while. Go get some coffee or something :)

Jaedson33 commented 7 years ago

I can't get coffee now because I'll sleep at midnight... So I'm going to play games ;)

Jaedson33 commented 7 years ago

Now I get this error :'( CMake is a pre-requisite to build this repository but it was not found on the path. Please install CMake from http://www.cmake.org/download/ and ensure it is on your path. Failed to generate native component build project! Command execution failed with exit code 1.

tintoy commented 7 years ago

Easy to fix - just download and install CMake from the path they give you in the error message. The build for .NET Core is...complicated. You're almost there ;-)

Jaedson33 commented 7 years ago

It's the problem, that don't say me what's the path!

anthonylangsworth commented 7 years ago

Download and install CMake from the URL provided. I cannot remember whether it adds itself to the path if not but, if not, add it to the PATH environment variable using control panel. The installation process should prompt and default the installation path so you know where to look for cmake.exe.

tintoy commented 7 years ago

I think they mean the PATH environment variable. You know how to edit that one (from memory, you can press WindowsKey+X, Y then go to Advanced System Settings, and finally click on Environment Variables)?

tintoy commented 7 years ago

Just checked - you can go to the start menu, and type "environment variable". It will show an item called "edit the system environment variables".

Jaedson33 commented 7 years ago

Now that's working =D Thanks a lot =D

Jaedson33 commented 7 years ago

I'm now geting other error 😢  Where is the testdotnetcli? The solution compile, but the tests don't start 😞

Jaedson33 commented 7 years ago

Do you have any ideas of how can I fix it? 😟 Is anybody online? 😭

Jaedson33 commented 7 years ago

And why there are some erros like "O tipo pré-definido "System.Object" não foi definido ou importado"?

peterwurzinger commented 7 years ago

@Jaedson33 To get the tests runnable you have to build the tests with the testdotnetcli first. Run build-tests -skiptests in your corefx-root. Then you should have a testdotnetcli and be able to run the tests. I can't really tell you much about the spanish or portuguese error :-(

Jaedson33 commented 7 years ago

@peterwurzinger I did it, and the result is: There isn't any testdotnetcli in the Tools folder :-(

tintoy commented 7 years ago

I think @anthonylangsworth ran into the same problem at one stage - I can't remember how we fixed it now. Have you tried running build-tests without -skip-tests? Does it successfully run tests in that case?

Jaedson33 commented 7 years ago

Yes, it run tests if I don't put -skip-tests, but it do it without Visual Studio 😕

Jaedson33 commented 7 years ago

I give up. I'll try to do it in some other way. 😠 But I don't know other way to do it 😅

tintoy commented 7 years ago

You can't use the test explorer in Visual Studio to run your tests. That's not supported (for now) as far as I am aware. Mark your test project as the startup project, then hit "Start without debugging". They should run correctly.

tintoy commented 7 years ago

(yeah, I know, it's non-obvious and frustrating, but there are bound to be teething problems when opening up such a large codebase and build system)

Jaedson33 commented 7 years ago

I also did that and it didn't work. I'm deleting the source code now.

Jaedson33 commented 7 years ago

While you fix the mistakes in these classes, I will implement what is still missing in my UWP. 😃

danmoseley commented 7 years ago

Alternatively if you grab https://github.com/dotnet/corefx/pull/16300 and do "msbuild /t:buildandtest /p:testdebugger=devenv" (assuming you are in a VS command prompt ie devenv is on the path) it will start the debugger when the tests start. You would have to manually open the test source file and set any breakpoints you desire. After the first time you do this, you can save the .sln file from VS for next time. You can also do future debugging just by hitting F5 in VS. No idea if this helps, but it's what I do.

anthonylangsworth commented 7 years ago

The version of dotnet.exe for running tests can be found under corefx\Tools\testdotnetcli\dotnet.exe. You'll need to run a full build.cmd to get it created. While there may be one or two warnings, ensure you get no errors. If you get errors and cannot fix them, list them here.

I could not get the tests working using msbuild on any platform. I ended up just using the post build event in Visual Studio to run the command line.

Jaedson33 commented 7 years ago

Do you think this project is ready to be used until the end of the month?

anthonylangsworth commented 7 years ago

I doubt it will be ready by the end of February 2017. Even if we get all a complete set of tests working, we still need to merge it back into the .Net Core master branch, requiring approval and review.

Jaedson33 commented 7 years ago

This is great News 😀 . Do you think it will be long before the PR to master branch be approved?

anthonylangsworth commented 7 years ago

Um, I just checked what I typed and I said (note the emphasis)

I doubt it will be ready by the end of February 2017

Unless you are happy with an unknown release date and unknown hurdles, I think you misread my comment.

Jaedson33 commented 7 years ago

At least now I know the release date isn't too far away.

anthonylangsworth commented 7 years ago

Merged the other branch. @tintoy will now rebase.