Skip to content

when "dotnet test" runs test projects in parallel mode, json file writes invalid #351

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

Closed
wants to merge 5 commits into from

Conversation

loning
Copy link

@loning loning commented Feb 21, 2019

dotnet test --no-build /p:CollectCoverage=true /p:CoverletOutputFormat='json%2copencover' /p:CoverletOutput="../results/coverage" /p:MergeWith="../results/coverage.json" /p:Exclude="[coverlet..tests?]"

../results/coverage.json may faces concurrency problem, so when making the file share mode be none, it will solve the problem

dotnet test --no-build /p:CollectCoverage=true /p:CoverletOutputFormat='json%2copencover' /p:CoverletOutput="../results/coverage" /p:MergeWith="../results/coverage.json" /p:Exclude="[coverlet.*.tests?]*"

../results/coverage.json may faces concurrency problem, so when making the file share mode be none, it will solve the problem
@codecov
Copy link

codecov bot commented Feb 21, 2019

Codecov Report

Merging #351 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #351      +/-   ##
=========================================
+ Coverage   87.05%   87.1%   +0.04%     
=========================================
  Files          31      31              
  Lines        3114    3125      +11     
=========================================
+ Hits         2711    2722      +11     
  Misses        403     403

@@ -32,7 +30,9 @@ public string Identifier
get { return _identifier; }
}

public Coverage(string module, string[] includeFilters, string[] includeDirectories, string[] excludeFilters, string[] excludedSourceFiles, string[] excludeAttributes, bool singleHit, string mergeWith, bool useSourceLink)
public Coverage(string module, string[] includeFilters, string[] includeDirectories, string[] excludeFilters,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please could you revert formatting to old one here and on other places?

return reader.ReadToEnd();
}
}
}, () => TimeSpan.FromMilliseconds(100), 5);
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli Feb 22, 2019

Choose a reason for hiding this comment

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

I'm not sure about max attempt, I would try for "more standard seconds" 10 or 30 /cc @petli @tonerdo


if (!string.IsNullOrEmpty(_mergeWith) && !string.IsNullOrWhiteSpace(_mergeWith) && File.Exists(_mergeWith))
{
string json = File.ReadAllText(_mergeWith);
var json = RetryHelper.Do(() =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please can you add some comment above like // Possible concurrency access to merge file between write/read on parallel testing here and below?

@tonerdo
Copy link
Collaborator

tonerdo commented Feb 23, 2019

@loning I'm not very clear the problem this PR is trying to solve

@loning
Copy link
Author

loning commented Feb 23, 2019

I have lots of unit test project under one solution, and when we call "dotnet test any.sln", the unit test project will run in parallel at the same time, when we use /p:CoverletOutput="../results/coverage" /p:MergeWith="../results/coverage.json", may two project will write to ../results/coverage.json at the same time, and also when other project want to merge, it may read a file not completed.

so my solution is to make the file not shared, and when we cannot access a file, we wait 100ms, cause 100ms is enough for another read or write

https://travis-ci.org/AElfProject/AElf/builds/497464201?utm_source=github_status&utm_medium=notification

@loning
Copy link
Author

loning commented Feb 23, 2019

I just found once error, and rerun the CI, disappeared. but I thought it's not hard to fix it

@MarcoRossignoli
Copy link
Collaborator

and when we call "dotnet test any.sln",

@loning run coverlet solution wide is not a supported scenario for now #350 (comment)

@loning
Copy link
Author

loning commented Feb 24, 2019

#351 (comment)

thanks,@MarcoRossignoli,but it seems worked well in our solution, we do not want to use shell to run test one by one

@MarcoRossignoli MarcoRossignoli added the enhancement General enhancement request label Jun 11, 2019
@MarcoRossignoli
Copy link
Collaborator

@loning for the moment I'll close this stale PR.
Even if you idea works well we should find a more robust way, we've an open issue on it #357

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

Successfully merging this pull request may close these issues.

3 participants