-
Notifications
You must be signed in to change notification settings - Fork 34
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
add git info to coveralls send #36
base: master
Are you sure you want to change the base?
Conversation
e0be469
to
5b72c31
Compare
Hi @markusn, Could you please take a look to this changes? You see, a lot of projects where used your great plugin have some issues in GitHub actions. This changes was retested use @RoadRunnr fork and working as expected for GH actions. Many thanks for your time. Regards, |
I’ll have a look sometime this week! |
b3003b5
to
3f27da5
Compare
Both are needed on GitHub actions. Coveralls seems to be doing magic things for the other supported CIs, but for GitHub actions that magic does not work. Instead we are supposed to supply complete information. That means the git info member on the API call and a flag_name member for parallel builds.
3f27da5
to
7c97500
Compare
Getting the parallel run to work on GitHub is a challenge. Add a small snippet to the README to explain how.
7c97500
to
4dc8b17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR! I have some comments that I'd like to have addressed before merging.
@@ -125,10 +125,12 @@ do_coveralls(ConvertAndSend, Get, GetLocal, MaybeSkip, Task) -> | |||
Report0 = | |||
#{service_job_id => ServiceJobId, | |||
service_name => ServiceName}, | |||
Report1 = collect_git_info(Report0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this step conditional on service being GitHub, running a shell command if the output isn't needed seems like a unnecessary.
Report | ||
end. | ||
|
||
collect_git_details(Id) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide test cases for this function!
collect_git_branch_details(#{head => Head#{id => Id}}). | ||
|
||
collect_git_branch_details(Git) -> | ||
{ok, Branch} = rebar_utils:sh("git rev-parse --abbrev-ref HEAD", []), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is specifically needed by GitHub actions, can't we use the GITHUB_SHA
variable or any other appropriate variable as specified at https://docs.github.com/en/actions/reference/environment-variables
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, by looking at the Erlang LS / Coveralls interaction (https://coveralls.io/github/erlang-ls/erlang_ls) it looks like the major issue with it is that somehow coverage for Pull Requests is correctly reported, but things do not work for master/main
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is specifically needed by GitHub actions, can't we use the
GITHUB_SHA
variable or any other appropriate variable as specified athttps://docs.github.com/en/actions/reference/environment-variables
?
The env variables only contain the commit SHA, but we need more than that. All fields in the git key need to be filled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, by looking at the Erlang LS / Coveralls interaction (https://coveralls.io/github/erlang-ls/erlang_ls) it looks like the major issue with it is that somehow coverage for Pull Requests is correctly reported, but things do not work for
master/main
.
Erlang LS is not using the changes from this PR, hence the wrong reporting.
Also, even when/if this PR is accepted, the GH Action in Erlang LS would need to be adjusted (see the README.md changes in the PR).
@RoadRunnr any comments on my feedback? |
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore'
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution}
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution}
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution} * Fix dialyzer warning: ------------------------------------------- src/metrics/prometheus_quantile_summary.erl 307: The created fun has no local return 346: The created fun has no local return 487: Function quantile_merge/2 has no local return 491: Record construction #quantile_estimator{samples_count::number(),data_count::'undefined',inserts_since_compression::'undefined',data::[#group{v::number(),g::number(),delta::number(),rank::number()}],invariant::fun()} violates the declared type of field data_count::number() and inserts_since_compression::number() -------------------------------------------
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution} * Fix dialyzer warning: ------------------------------------------- src/metrics/prometheus_quantile_summary.erl 307: The created fun has no local return 346: The created fun has no local return 487: Function quantile_merge/2 has no local return 491: Record construction #quantile_estimator{samples_count::number(),data_count::'undefined',inserts_since_compression::'undefined',data::[#group{v::number(),g::number(),delta::number(),rank::number()}],invariant::fun()} violates the declared type of field data_count::number() and inserts_since_compression::number() -------------------------------------------
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution} * Fix dialyzer warning: ------------------------------------------- src/metrics/prometheus_quantile_summary.erl 307: The created fun has no local return 346: The created fun has no local return 487: Function quantile_merge/2 has no local return 491: Record construction #quantile_estimator{samples_count::number(),data_count::'undefined',inserts_since_compression::'undefined',data::[#group{v::number(),g::number(),delta::number(),rank::number()}],invariant::fun()} violates the declared type of field data_count::number() and inserts_since_compression::number() -------------------------------------------
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Added 'elvis' call to GH Actions * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution} * Fix dialyzer warning: ------------------------------------------- src/metrics/prometheus_quantile_summary.erl 307: The created fun has no local return 346: The created fun has no local return 487: Function quantile_merge/2 has no local return 491: Record construction #quantile_estimator{samples_count::number(),data_count::'undefined',inserts_since_compression::'undefined',data::[#group{v::number(),g::number(),delta::number(),rank::number()}],invariant::fun()} violates the declared type of field data_count::number() and inserts_since_compression::number() -------------------------------------------
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Added 'elvis' call to GH Actions * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution} * Fix dialyzer warning: ------------------------------------------- src/metrics/prometheus_quantile_summary.erl 307: The created fun has no local return 346: The created fun has no local return 487: Function quantile_merge/2 has no local return 491: Record construction #quantile_estimator{samples_count::number(),data_count::'undefined',inserts_since_compression::'undefined',data::[#group{v::number(),g::number(),delta::number(),rank::number()}],invariant::fun()} violates the declared type of field data_count::number() and inserts_since_compression::number() -------------------------------------------
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Added 'elvis' call to GH Actions * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix error for GH Action when tests running - {'EXIT',nodistribution} * Fix dialyzer warning: ------------------------------------------- src/metrics/prometheus_quantile_summary.erl 307: The created fun has no local return 346: The created fun has no local return 487: Function quantile_merge/2 has no local return 491: Record construction #quantile_estimator{samples_count::number(),data_count::'undefined',inserts_since_compression::'undefined',data::[#group{v::number(),g::number(),delta::number(),rank::number()}],invariant::fun()} violates the declared type of field data_count::number() and inserts_since_compression::number() -------------------------------------------
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Added 'elvis' call to GH Actions * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore'
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Added 'elvis' call to GH Actions * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore'
Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Added 'elvis' call to GH Actions * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore'
* Added GH Action Purposes of changes: * Provided base GitHub Actions * Added coveralls: used fork till merge/release markusn/coveralls-erl#36 * Added 'elvis' call to GH Actions * Updated 'README': added GH Badges * Remove '.travis.yml' * Update 'rebar.config.script' based on GH Actions * Add 'rebar3' to '.gitignore' * Fix dialyzer warning src/metrics/prometheus_quantile_summary.erl 307: The created fun has no local return 346: The created fun has no local return 487: Function quantile_merge/2 has no local return 491: Record construction #quantile_estimator{samples_count::number(),data_count::'undefined',inserts_since_compression::'undefined',data::[#group{v::number(),g::number(),delta::number(),rank::number()}],invariant::fun()} violates the declared type of field data_count::number() and inserts_since_compression::number() * Fix error for GH Action when tests running In GH Action was provided error '{'EXIT',nodistribution}' Looks like 'epmd -daemon' is not working as expected for containers Start use 'erl -sname test' instead of 'epmd -daemon' will fix this issue
It contains the following patch to fix the send of Git info: markusn/coveralls-erl#36 It also fixes the send of source code.
This is needed for output on GitHub Actions to correctly show up on coveralls.
might also fix #34.