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

Assertion failure in 2.5.2 #719

Closed
jskeet opened this issue Sep 19, 2016 · 13 comments
Closed

Assertion failure in 2.5.2 #719

jskeet opened this issue Sep 19, 2016 · 13 comments

Comments

@jskeet
Copy link

jskeet commented Sep 19, 2016

This is using 2.5.2 from the zip file, against a dotnet cli project. (It's not fully clear to me whether that's supported, but at least the result could be prettier...)

Running docfx metadata -f path/to/docfx.json I get a Watson screen as below:

docfx assertion

Two problems:

  • I have no idea what the failure is here
  • Coming up as an assertion failure window rather than an error appearing on the console is worrying; I don't know what would happen if this occurred in CI, for example. (It also means I can't copy/paste the text etc.)

The same project works fine in 2.4, btw.

@vwxyzh
Copy link
Contributor

vwxyzh commented Sep 19, 2016

Only 4 commit is related from v2.4:
Update to support .NET Core (#656) 821dd49
Add information for metadata fail. (#665) 6d18d0a
update documentation for attribute filter (#686) ef2c136
Remove *.csproj and packages.config (#706) f8b3945

@jskeet
Copy link
Author

jskeet commented Sep 19, 2016

Hmm. I'll try with the zip files from each of the releases then (2.4.0, 2.5.0, 2.5.1) to see where it came from. I was using DNX for 2.4.0, so I don't know whether there's a significant difference in the code there...

@jskeet
Copy link
Author

jskeet commented Sep 19, 2016

Okay, the zip file for 2.4.0 doesn't work for me because it doesn't handle project.json files, but 2.5.0 fails in the same way as the original post :(

@vwxyzh
Copy link
Contributor

vwxyzh commented Sep 20, 2016

We found something wrong in project.json:

    "Release": {
      "buildOptions": {
        "define": [ "DEBUG", "TRACE" ],
        "optimize": true
      }
    }

@qinezh Release build should not contain DEBUG.

@qinezh
Copy link
Contributor

qinezh commented Sep 20, 2016

The issue should be fixed in latest release DocFX, could you have a try?

@jskeet
Copy link
Author

jskeet commented Sep 20, 2016

Nope, I'm afraid I still get the same assertion failure with 2.5.3 :(

@nothrow
Copy link
Contributor

nothrow commented Sep 21, 2016

I have reproducible sample here. For me, issue reproduces when I have interface method with template.

Steps to reproduce:

docfx init -q

docfx.json:

{
"metadata": [
    {
      "src": [
        {
          "files": [
            "src/*.cs"
          ], ... 

add to src\ following:

IInterface.cs:

namespace A {
  public interface IInterface
  {
    /// <summary>
    /// Summary
    /// </summary>
    void Function<T>();
  }
}

Implementation.cs:

namespace A {
  public class Implementation : IInterface
  {
    public void Function<T>()
    {
    }
  }
}

2.5.3 version.

The assertion that fails is Debug.Assert(source == null || target == null || Nullable.Equals(source, target));.

I tried to create unit test with repro, but the Debug.Assert does not fail it, so the test is green.

The test is at https://github.com/nothrow/docfx/commit/26787fd579a0c1625d3c2940075d6bfdc38637df

@qinezh
Copy link
Contributor

qinezh commented Sep 22, 2016

The issue may caused by that dotnet publish also need specify build configuration

@qinezh
Copy link
Contributor

qinezh commented Sep 22, 2016

New release updated

@jskeet
Copy link
Author

jskeet commented Sep 22, 2016

Hooray - this does seem to have fixed it, thanks.

@jskeet jskeet closed this as completed Sep 22, 2016
@qinezh
Copy link
Contributor

qinezh commented Sep 22, 2016

@jskeet great to hear that 😄

@nothrow Thank you to repro it, we will fix the case later. BTW, could you open a pull request to include the unit test? It seems worth to add it in DocFX.

@nothrow
Copy link
Contributor

nothrow commented Sep 22, 2016

@qinezh, #728

Just one comment: I think that the fix in v2.5.4 just hides the issue. It was not regression, I reproduced the issue even before v2.5.2, when compiled with DEBUG.

My .02$ advice would be - don't use Debug.Assert, just throw, even in release, or lot of problems may be there left unnoticed.

@qinezh
Copy link
Contributor

qinezh commented Sep 22, 2016

Debug.Assert in this case is by design as I know, @vwxyzh Could you explain it to @nothrow ?

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

No branches or pull requests

4 participants