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

Calculate Coverage #113

Merged
merged 19 commits into from
Oct 24, 2019
Merged

Calculate Coverage #113

merged 19 commits into from
Oct 24, 2019

Conversation

shargon
Copy link
Member

@shargon shargon commented Oct 18, 2019

@shargon
Copy link
Member Author

shargon commented Oct 18, 2019

@erikzhang We need to configure the environment variable for codecov as we did in the past in other neo projects. Could you do it please?

@codecov-io
Copy link

codecov-io commented Oct 19, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@502c0ab). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #113   +/-   ##
=========================================
  Coverage          ?   44.06%           
=========================================
  Files             ?       12           
  Lines             ?     3499           
  Branches          ?        0           
=========================================
  Hits              ?     1542           
  Misses            ?     1957           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 502c0ab...8f1e91e. Read the comment docs.

@shargon
Copy link
Member Author

shargon commented Oct 19, 2019

Done, the problem is that SmartContract.Framework could not be instrumented by the coverage engine because it's executed on the VM and the test will fail. Although i fix in the compiler the instrumentation issues, it never be executed because is executed inside the neo VM.

So the coverage is only for Neo.Compiler.MSIL

@shargon
Copy link
Member Author

shargon commented Oct 22, 2019

We should remove OSX here too? or not because is not a library? @erikzhang

@erikzhang
Copy link
Member

Just remove it.

@shargon
Copy link
Member Author

shargon commented Oct 22, 2019

Ok, then it's done

@shargon shargon changed the title Calculate Coverage Calculate Coverage And Upgrade dependencies and target frameworks Oct 23, 2019
@erikzhang
Copy link
Member

I think there is no need to upgrade the target frameworks of these projects.

@shargon
Copy link
Member Author

shargon commented Oct 24, 2019

Is not better to have all the projects unified?

@erikzhang
Copy link
Member

When we are going to upgrade the target framework, either because the new framework fixes a serious bug or because we want to use the new features in the new framework. Otherwise, the lower the version of the framework, the better, because there will be better compatibility.

@shargon
Copy link
Member Author

shargon commented Oct 24, 2019

So do you think that we should only upgrade the unit test? or none of them?

@erikzhang
Copy link
Member

None of them.

@shargon shargon changed the title Calculate Coverage And Upgrade dependencies and target frameworks Calculate Coverage Oct 24, 2019
@shargon
Copy link
Member Author

shargon commented Oct 24, 2019

Then leave it only for coverage

@shargon shargon merged commit 01a2dec into master Oct 24, 2019
@shargon shargon deleted the shargon-patch-1 branch October 24, 2019 09:09
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

Successfully merging this pull request may close these issues.

None yet

3 participants