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

Adding VectorCAST coverage #105

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Conversation

TimSVector
Copy link

Adding MC/DC, Function, and Function Call coverage. Results being shown in the coverage report and associated graphs. Method coverage not working for some reason.

Testing done

Tested with an expanded Cobertura xml file. Run through multiple configurations on Jenkins server with varying coverage configurations

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Open a fork of the repository
    Options
  2. Ensure that the pull request title represents the desired changelog entry
    Options
  3. Please describe what you did
    Options
  4. Link to relevant issues in GitHub or Jira
    Options
  5. Link to relevant pull requests, esp. upstream and downstream changes
    Options
  6. Ensure you have provided tests - that demonstrates feature works or fixes the issue
    Options

@uhafner uhafner added the feature New features label May 8, 2024
@TimSVector
Copy link
Author

Not sure how to fix this the missing comma affer "id" : "error-prone"

Quality Monitor PR / Build, test and monitor quality on Ubuntu (pull_request_target) Failing after 3m

"tools": [
  {
    "id": "spotbugs",
    "sourcePath": "src/main/java",
    "pattern": "**/target/spotbugsXml.xml"
  },
  {
    "id": "error-prone"
    "pattern": "**/maven.log"
  }

@TimSVector
Copy link
Author

Not sure how to fix this the missing comma affer "id" : "error-prone"

Quality Monitor PR / Build, test and monitor quality on Ubuntu (pull_request_target) Failing after 3m

"tools": [
  {
    "id": "spotbugs",
    "sourcePath": "src/main/java",
    "pattern": "**/target/spotbugsXml.xml"
  },
  {
    "id": "error-prone"
    "pattern": "**/maven.log"
  }

@uhafner - do you know how to clear this error? I don't know what I've done in the branch to create it. I'd like to move on to the task of fixing the coverage-plugin changes that are pending this artifact

@uhafner
Copy link
Member

uhafner commented May 16, 2024

I think I fixed that in main.

@uhafner
Copy link
Member

uhafner commented May 16, 2024

It seems that you did commit your changes in main. This is not ideal. You should always create changes in a separate branch. Then it is much easier to update from the upstream repository.

@TimSVector
Copy link
Author

TimSVector commented May 16, 2024 via email

@uhafner
Copy link
Member

uhafner commented May 16, 2024

I thought you requested a fork and a PR. I can redo if you’d like.

No, a fork is correct. You just never use the main branch even in your fork. Otherwise you cannot keep your forked main branch in sync with the main branch of the upstream, see https://gist.github.com/Chaser324/ce0505fbed06b947d962

You see the effect of not creating a branch now in the diff: https://github.com/jenkinsci/coverage-model/pull/105/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5

Now there are many unrelated changes visible...

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this pull request, everything is well written and tested!

The only bigger thing is the additional FUNCTION metric. I would be very happy if we can achieve the same results with the existing METHOD metric.

src/main/java/edu/hm/hafner/coverage/FileNode.java Outdated Show resolved Hide resolved
src/main/java/edu/hm/hafner/coverage/FileNode.java Outdated Show resolved Hide resolved
src/test/java/edu/hm/hafner/coverage/FileNodeTest.java Outdated Show resolved Hide resolved
Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing all those tasks! The only thing I would like to change is the duplicate enum FUNCTION vs. METHOD. Do you think that we can eliminate it? I think I almost got rid of it a couple of weeks ago but then dropped my changes... If you need help, I can push a draft of these changes again...

@TimSVector
Copy link
Author

Thanks for fixing all those tasks! The only thing I would like to change is the duplicate enum FUNCTION vs. METHOD. Do you think that we can eliminate it? I think I almost got rid of it a couple of weeks ago but then dropped my changes... If you need help, I can push a draft of these changes again...

Please push any suggested changes for this. I cannot figure it out. I think what's tripping me up is Method depends on Children and Function is a Value (2/4 50%)

    FUNCTION(new ValuesAggregator()),
    METHOD(new LocOfChildrenEvaluator()),

@uhafner
Copy link
Member

uhafner commented Jun 4, 2024

Yes, this exactly was the problem. I fixed that code now. When a local value is present, then we do not need to derive the "container" metrics from the line coverage.

I pushed my changes here: d9a047c

If you prefer, I can push the changes back to your repo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New features
Projects
None yet
2 participants