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

Set the coverageDestPath for all files to the root folder #1812

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wilzbach
Copy link
Member

@wilzbach wilzbach commented Jul 3, 2017

Follow-up to #1810 (comment)

Not sure whether this helps to improve things or if CodeCov already merges the lst files automagically.

@s-ludwig
Copy link
Member

s-ludwig commented Jul 3, 2017

At least for "dub test" it could be really useful to let DUB get an optional (merge) destination path. It could then inject a shared static this in the compiled code that does the necessary druntime fiddling. I guess that could save quite some manual work in this area. Of course that would potentially still leave the separate tests, if CC indeed doesn't merge those results automatically...

@wilzbach
Copy link
Member Author

wilzbach commented Jul 3, 2017

I guess that could save quite some manual work in this area

Or we simply could push for a -cov-dest and -cov-merge CLI flags at DMD - these hypothetical flags would save us most of the troubles here, but I am not sure whether this will happen in the near future if neither of us isn't pushing it.

@s-ludwig
Copy link
Member

s-ludwig commented Jul 3, 2017

True, that would even be better.

@s-ludwig
Copy link
Member

s-ludwig commented Jul 5, 2017

Although for some reason the CodeCov status isn't shown here, this actually made the coverage drop by 2 percent points for some reason. This is definitely too hard to get right currently.

@s-ludwig
Copy link
Member

s-ludwig commented Jul 5, 2017

Source. https://codecov.io/gh/rejectedsoftware/vibe.d/pulls
for some reason clicking on the PR there results in 404, although CodeCov is authorized for this repo.

@wilzbach
Copy link
Member Author

wilzbach commented Jul 5, 2017

Although for some reason the CodeCov status isn't shown here, this actually made the coverage drop by 2 percent points for some reason. This is definitely too hard to get right currently.

Yep, what do you propose? Adding the runtime fiddling by default in DUB's unittest-cov mode?

for some reason clicking on the PR there results in 404, although CodeCov is authorized for this repo.

I have already written the CodeCov support about this. At least this seems to be a problem on their side.

@wilzbach
Copy link
Member Author

FYI there's dlang/druntime#1916 now

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.

2 participants