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

Transition the coverage-linux64 pipeline to Buildkite #41238

Merged
merged 4 commits into from
Jun 20, 2021

Conversation

DilumAluthge
Copy link
Member

No description provided.

@DilumAluthge DilumAluthge added the ci Continuous integration label Jun 16, 2021
@DilumAluthge DilumAluthge force-pushed the dpa/buildkite-coverage-linux64 branch 4 times, most recently from 9f6d76e to b4375c8 Compare June 16, 2021 06:13
@DilumAluthge DilumAluthge changed the title [WIP] Transition coverage-linux64 to Buildkite [WIP] Transition the coverage-linux64 pipeline to Buildkite Jun 16, 2021
@DilumAluthge DilumAluthge force-pushed the dpa/buildkite-coverage-linux64 branch 7 times, most recently from 7171760 to 1d8fc2d Compare June 16, 2021 17:47
@DilumAluthge
Copy link
Member Author

I've temporarily disabled the commit statuses. We can re-enable them right before we merge this PR.

In the meantime, go here to view statuses and build logs: https://buildkite.com/julialang/julia-coverage-linux64

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Jun 16, 2021

@vtjnash Any idea why the coverage is lower here compared to the Buildbot approach? This PR calculates coverage at 79% (58650/73776), but Codecov (which was uploaded from Buildbot) calculates coverage at 88% (65731/75061).

This PR runs the tests in parallel. Could that be the sole reason for the discrepancy, or is there something else that I'm missing?

@vtjnash
Copy link
Member

vtjnash commented Jun 17, 2021

You should be able to go to codecov or coveralls and compare the two commits directly

@DilumAluthge
Copy link
Member Author

You should be able to go to codecov or coveralls and compare the two commits directly

Ah, we'll have to add a Codecov token to Buildkite first - I'll need @staticfloat's help with that.

@DilumAluthge DilumAluthge force-pushed the dpa/buildkite-coverage-linux64 branch from 24b8553 to 6aa65ed Compare June 18, 2021 05:36
@DilumAluthge
Copy link
Member Author

Hmmm, looks like it's failing: https://buildkite.com/julialang/julia-coverage-linux64/builds/68

@staticfloat staticfloat force-pushed the dpa/buildkite-coverage-linux64 branch from 42ccc26 to 2610b80 Compare June 18, 2021 16:47
@staticfloat
Copy link
Member

So I have updated this to now correctly build Julia, and I've also updated the buildkite webui settings to not build in reaction to GH events, but to instead build on a nightly schedule.

@staticfloat
Copy link
Member

The build is proceeding here: https://buildkite.com/julialang/julia-coverage-linux64/builds/85#8b067d64-5d75-4139-b75a-81289b567f47

@DilumAluthge
Copy link
Member Author

I've also updated the buildkite webui settings to not build in reaction to GH events, but to instead build on a nightly schedule.

Nice! This sounds good.

If I want to e.g. trigger a manual job, based on the latest commit to master, without waiting for the next nightly build, will I still be able to trigger a manual coverage job through the Buildkite web UI?

@DilumAluthge
Copy link
Member Author

┌ Info:
--
  | │   ncores = 1
  | │   Sys.CPU_THREADS = 128
  | └   Threads.nthreads() = 1

We should set ncores to something more useful. 16?

@staticfloat
Copy link
Member

will I still be able to trigger a manual coverage job through the Buildkite web UI?

Yep; just click the big green "new build" button in the top right of the pipeline UI.

We should set ncores to something more useful. 16?

Hmmm, yeah. What's the best way to do that? Define JULIA_NUM_THREADS globally, for all jobs? I guess if we really want to run a job single-threaded we can override that, and this allows us to scale our jobs based on the agent itself defining how much parallelism it can sustain.

@DilumAluthge
Copy link
Member Author

Hmmm, yeah. What's the best way to do that? Define JULIA_NUM_THREADS globally, for all jobs? I guess if we really want to run a job single-threaded we can override that, and this allows us to scale our jobs based on the agent itself defining how much parallelism it can sustain.

Sounds good to me. Except I would define both JULIA_NUM_THREADS and JULIA_CPU_THREADS.

@staticfloat
Copy link
Member

Looks like it worked!

@DilumAluthge
Copy link
Member Author

Now we just need to uncommon the last line (# Coverage.Codecov.submit_local(fcs)) and make sure the upload works.

@DilumAluthge DilumAluthge changed the title [WIP] Transition the coverage-linux64 pipeline to Buildkite [WIP]Transition the coverage-linux64 pipeline to Buildkite Jun 18, 2021
@DilumAluthge DilumAluthge changed the title [WIP]Transition the coverage-linux64 pipeline to Buildkite Transition the coverage-linux64 pipeline to Buildkite Jun 18, 2021
@staticfloat staticfloat force-pushed the dpa/buildkite-coverage-linux64 branch from 81f6a44 to 4b4e390 Compare June 19, 2021 17:35
@staticfloat
Copy link
Member

I rebased onto latest master and the tests broke. I regret everything.

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Jun 19, 2021

I restarted the failing Windows Buildbots, and now all the Buildbots are passing.

Also, I ran a new Buildkite coverage-linux64 job, and if you look at the logs (https://buildkite.com/julialang/julia-coverage-linux64/builds/89), everything passed, including both the upload to Coveralls and the upload to Codecov.

It does look like coverage dropped approximately 6%. I'm guessing that is because of the tests being run in parallel.

In a future PR, I'll add a step that runs a subset of the tests in serial (specifically, the subset on which coverage dropped) so we can get coverage back up.

But I don't think that needs to block this PR.

@DilumAluthge DilumAluthge requested a review from staticfloat June 19, 2021 22:24
@DilumAluthge
Copy link
Member Author

From my point of view, this PR is good to go now.

@staticfloat staticfloat merged commit 9d5f31e into master Jun 20, 2021
@staticfloat staticfloat deleted the dpa/buildkite-coverage-linux64 branch June 20, 2021 07:09
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
)

* Transition the `coverage-linux64` pipeline to Buildkite

* Simplify, run inside of a sandbox

* Upload coverage reports to Codecov and Coveralls

* Add `COVERALLS_TOKEN`

Co-authored-by: Elliot Saba <[email protected]>
@KristofferC KristofferC added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Jul 19, 2021
KristofferC pushed a commit that referenced this pull request Jul 19, 2021
* Transition the `coverage-linux64` pipeline to Buildkite

* Simplify, run inside of a sandbox

* Upload coverage reports to Codecov and Coveralls

* Add `COVERALLS_TOKEN`

Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 9d5f31e)
KristofferC pushed a commit that referenced this pull request Jul 19, 2021
* Transition the `coverage-linux64` pipeline to Buildkite

* Simplify, run inside of a sandbox

* Upload coverage reports to Codecov and Coveralls

* Add `COVERALLS_TOKEN`

Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 9d5f31e)
KristofferC pushed a commit that referenced this pull request Jul 20, 2021
* Transition the `coverage-linux64` pipeline to Buildkite

* Simplify, run inside of a sandbox

* Upload coverage reports to Codecov and Coveralls

* Add `COVERALLS_TOKEN`

Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 9d5f31e)
@KristofferC KristofferC mentioned this pull request Jul 26, 2021
75 tasks
KristofferC pushed a commit that referenced this pull request Aug 31, 2021
* Transition the `coverage-linux64` pipeline to Buildkite

* Simplify, run inside of a sandbox

* Upload coverage reports to Codecov and Coveralls

* Add `COVERALLS_TOKEN`

Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 9d5f31e)
KristofferC pushed a commit that referenced this pull request Sep 3, 2021
* Transition the `coverage-linux64` pipeline to Buildkite

* Simplify, run inside of a sandbox

* Upload coverage reports to Codecov and Coveralls

* Add `COVERALLS_TOKEN`

Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 9d5f31e)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Sep 7, 2021
@vtjnash
Copy link
Member

vtjnash commented Sep 9, 2021

Why is coverage being miscomputed since this PR was merged? https://codecov.io/gh/JuliaLang/julia/compare/6d2c0a7766142cbeb52c21494484038a558d9b33...9d5f31e9231c1d77e24ee820908e32f559e23057

It seems like the wrong flags must be getting passed to the test runners?

@DilumAluthge
Copy link
Member Author

@DilumAluthge
Copy link
Member Author

Hmmm. Here is one possibility.

So, we need to make sure we pass certain command-line flags, right? Including the following:

  1. --code-coverage=all
  2. --sysimage-native-code=no

Now, when we run the tests in parallel, we are using Base.runtests, which uses Distributed to launch workers. What if the above flags are not getting correctly forwarded to the Distributed workers?

@DilumAluthge
Copy link
Member Author

For example, I don't think Distributed is correctly forwarding the --sysimage-native-code=no flag.

julia> Base.JLOptions().use_sysimage_native_code
0

julia> Distributed.@distributed vcat for i = 1:20
       Base.JLOptions().use_sysimage_native_code
       end
20-element Vector{Int8}:
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1
 1

@vtjnash
Copy link
Member

vtjnash commented Sep 9, 2021

I think test/testenv.jl forwards different flags? Also recommended is --code-coverage=lcov/tracefile-%p.info

c.f. https://github.com/JuliaCI/CoverageBase.jl/blob/master/examples/run_coverage_codecov.sh#L16

staticfloat pushed a commit that referenced this pull request Dec 23, 2022
* Transition the `coverage-linux64` pipeline to Buildkite

* Simplify, run inside of a sandbox

* Upload coverage reports to Codecov and Coveralls

* Add `COVERALLS_TOKEN`

Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 9d5f31e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies ci Continuous integration system:linux Affects only Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants