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

Complex branches are reported incorrectly #79

Closed
SteveGilham opened this issue May 3, 2018 · 6 comments
Closed

Complex branches are reported incorrectly #79

SteveGilham opened this issue May 3, 2018 · 6 comments
Labels
bug Something isn't working tenet-coverage Issue related to possible incorrect coverage

Comments

@SteveGilham
Copy link

SteveGilham commented May 3, 2018

This is the same code as for OpenCover issue 819
819-bis
The only branch point reported for this line is

                <BranchPoint vc="9" upsid="198" ordinal="3" path="" offset="3" offsetend="3" sl="198" fileid="4" />

when there are five visible (seven in the underlying IL) ways of proceeding

I note in passing that the offset values here are not the IL offset values of the branch and its terminus as generated by opencover.

@SteveGilham
Copy link
Author

Actually, it seems that switch instructions are not being recognised as branches, either as in

    public enum Scope
    {
        None = 0,
        Some,
        Many
    }
...
            switch (arg)
            {
                case Scope.None: ...
                case Scope.Some: ...
                case Scope.Many: ...
            }

or in F# match expressions like

type internal FilterClass =
  | File of Regex
  | Assembly of Regex
  | Module of Regex
  | Type of Regex
  | Method of Regex
  | Attribute of Regex
...
    match filter with
    | File name -> ...
    | Assembly name -> ...
    | Module name -> ...
    | Type name -> ...
    | Method name -> ...
    | Attribute name -> ...

@hunterjm
Copy link
Contributor

hunterjm commented May 4, 2018

Branch enhancements are in #73. It uses the same logic OpenCover uses for determining branches, so it will still only report 5 vs the 7, but 5 > 1. It also fixes the switches not showing as branches.

@SteveGilham
Copy link
Author

OpenCover does report 5 branches in this case, but they're the wrong 5. The decompiled code looks like

			if (!(message is Message.Item)) <= not this
			{
				if (!(message is Message.Finish)) <= nor this
				{
                                     ...
                                     return;
				}
				Message.Finish finish = (Message.Finish)msg;
				switch (finish.item1.Tag) <=but  5 paths from this 
				{
				default: // Cases 0 and 1
                                     ...
                                     return;
				case 2: // Pause
                                     ...
                                     return;
				case 3: // Resume
                                     ...
                                     return;
				}
			}
                        ...
                        return;

and it detects cases 0, 1 and out-of-band as distinct routes to the default case (see OpenCover issue #786) , and doesn't detect the if branches.

@MarcoRossignoli MarcoRossignoli added bug Something isn't working tenet-coverage Issue related to possible incorrect coverage labels Aug 16, 2019
@MarcoRossignoli MarcoRossignoli added the up-for-grabs Good issue for contributors label Sep 26, 2019
@MarcoRossignoli MarcoRossignoli removed the up-for-grabs Good issue for contributors label Feb 21, 2021
@MarcoRossignoli
Copy link
Collaborator

@SteveGilham this issue is very old, is this issue still present? Can you give it a try with last version?

@SteveGilham
Copy link
Author

Between changes in the compiler and the tooling, this issue no longer reproduces. The example code was like this
Coverlet79.zip

Now the decompiled code looks almost the same (default shown last in the decompilation)

	if (!(msg is Message.Item)) // offset = 10
	{
		if (!(msg is Message.Finish))  // offset = 18
		{
			...
			return;
		}
		Message.Finish finish = (Message.Finish)msg;
		switch (finish.item1.Tag) // offset = 58
		{
		case 1: // Pause
			...
			return;
		case 2: // Resume
			...
			return;
		default:  
			...
			return;
		}
	}
	...
	return;

and 8 branches are shown

                <BranchPoint vc="0" uspid="18" ordinal="0" path="0" offset="10" offsetend="12" sl="18" fileid="2" />
                <BranchPoint vc="0" uspid="18" ordinal="2" path="0" offset="18" offsetend="20" sl="18" fileid="2" />
                <BranchPoint vc="1" uspid="18" ordinal="1" path="1" offset="10" offsetend="29" sl="18" fileid="2" />
                <BranchPoint vc="0" uspid="18" ordinal="3" path="1" offset="18" offsetend="38" sl="18" fileid="2" />
                <BranchPoint vc="0" uspid="18" ordinal="6" path="2" offset="58" offsetend="138" sl="18" fileid="2" />
                <BranchPoint vc="0" uspid="18" ordinal="7" path="3" offset="58" offsetend="156" sl="18" fileid="2" />
                <BranchPoint vc="0" uspid="18" ordinal="4" path="0" offset="58" offsetend="174" sl="18" fileid="2" />
                <BranchPoint vc="0" uspid="18" ordinal="5" path="1" offset="58" offsetend="174" sl="18" fileid="2" />

2 per if, 3 switch cases and path 0 being the unobtainable "do not jump" case

@MarcoRossignoli
Copy link
Collaborator

Thx for testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tenet-coverage Issue related to possible incorrect coverage
Projects
None yet
Development

No branches or pull requests

3 participants