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

use master branch of dependency packages in nightly test stage #805

Closed
johnnychen94 opened this issue May 9, 2019 · 11 comments
Closed

use master branch of dependency packages in nightly test stage #805

johnnychen94 opened this issue May 9, 2019 · 11 comments

Comments

@johnnychen94
Copy link
Member

johnnychen94 commented May 9, 2019

It's a good idea to catch as many errors as possible before upstream dependency repos tag releases.

With a quite trivial script and appropriate Travis configuration, we can make this idea work, i.e., use master branch of dependency packages (ImageCore, ImageDistances etc) in nightly test.

How to:

  1. set Travis environment variable in https://travis-ci.org/JuliaImages/PACKAGE.jl/settings

SCRIPT_URL=https://gist.githubusercontent.com/johnnychen94/720900fd9e44395beaa0704770c91a36/raw/74f1a02f420a3ccd64b56cfc14111d170f559330/checkout_master.jl

(limitation: only repos that have valid Project.toml are supported by this simple script)

  1. update .travis.yml

(P.S. If there's a better(more concise) configuration, please tell me.)

travis configuration example
language: julia
os:
  - linux
  - osx
julia:
  - 1.0
  - 1.1
  - nightly
env:
  - USE_MASTER=true
  - USE_MASTER=""

notifications:
  email: false
matrix:
  exclude:
    - julia: 1.0
      env: USE_MASTER=true
    - julia: 1.1
      env: USE_MASTER=true
    - julia: nightly
      env: USE_MASTER=""
  allow_failures:
  - julia: nightly

before_script:
  - if [ ! -z "$USE_MASTER" ]; then curl $SCRIPT_URL -o install_master.jl; julia install_master.jl;  fi

after_success:
  - julia -e 'using Pkg; Pkg.add("Coverage"); using Coverage; Codecov.submit(process_folder())'

jobs:
  include:
    - stage: "Documentation"
      julia: 1.0
      os: linux
      env: USE_MASTER=""
      script:
        - julia -e 'using Pkg; Pkg.add("Documenter")'
        - julia -e 'using ImageNoise; ENV["DOCUMENTER_DEBUG"] = "true"; include(joinpath("docs","make.jl"))'
      after_success: skip

Demo:
https://travis-ci.org/johnnychen94/ImageNoise.jl/builds/530298110

@timholy
Copy link
Member

timholy commented May 11, 2019

Really interesting idea! When do you imagine running this? Only when people submit PRs to Images.jl itself? The obvious concern is that this may not catch dependency-PRs that merge and then quickly tag.

I suppose the alternative is to run the Images tests on each PR for each dependency? At one point, JuliaInterpreter also ran Debugger's tests: JuliaDebug/JuliaInterpreter.jl@ee0cd22. My main worry is making the whole dev pipeline slower, but there is little doubt that it would be safer.

@johnnychen94
Copy link
Member Author

johnnychen94 commented May 12, 2019

Regardless of the slower dev pipeline, actually, these two approaches are quite different; my approach here is quite passive and defensive, while yours is active and aggressive.

For example:
It's not practical to add ImageCore.jl test in Colors.jl and FixedPointNumbers.jl, but we can test ImageCore.jl with master branches of Colors.jl and FixedPointNumbers.jl.

The obvious concern is that this may not catch dependency-PRs that merge and then quickly tag.

Afterall, I guess it's exactly the purpose of the nightly test? A daily cron job in Images.jl might be necessary though.


I agree that it would be quite reasonable to adding Images.jl test in ImageCore.jl.

@timholy
Copy link
Member

timholy commented May 12, 2019

Afterall, I guess it's exactly the purpose of the nightly test? A daily cron job in Images.jl might be necessary though.

Yes, that would catch a lot. But not infrequently, I merge something and tag it within a few minutes. Those are usually simple bugfixes, so may not be as dangerous as other types of changes that may have a longer latency.

Perhaps the most useful time to run some additional tests would be when a new version is being registered. It would be cool if we could supply a custom test script to JuliaRegistries/General, that would launch additional tests on Travis (https://github.com/JuliaRegistries/General/issues/684). In the case where it's just a list of packages whose tests need running, the actual implementation would be fairly simple, I think, just an if isfile("ExtraTests.toml") ... end block here.

@johnnychen94
Copy link
Member Author

Shall I continue this issue? I think this issue doesn't conflict with https://github.com/JuliaRegistries/General/issues/684 and no additional tests are required.

@timholy
Copy link
Member

timholy commented May 16, 2019

If we fix it in the registry, is this still needed?

@johnnychen94
Copy link
Member Author

johnnychen94 commented May 16, 2019

Although it might be infrequent, it does help catch errors before dependent packages(e.g., FixedPointNumbers) registering new releases.

@timholy
Copy link
Member

timholy commented May 16, 2019

If I understand correctly, you're worried about a bug on the master branch of A that breaks a registered version of B. Fixing it in the registry will catch the error when A is registered, but you're hoping to catch it even earlier? Note that merely registering a new version of B will not incorporate the breakage in A, so I think this issue only affects people who have devved the package.

@johnnychen94
Copy link
Member Author

johnnychen94 commented May 16, 2019

Yes, that's exactly my purpose

For example, JuliaRegistries/General#642 is caught by JuliaImages/ImageCore.jl#78

I think this issue only affects people who have devved the package.

That's why we set allow_failures: nightly, it doesn't affect normal dev-ops, just help to aggressively catch more errors.

@timholy
Copy link
Member

timholy commented May 16, 2019

There is merit here. The particular bug you flagged has other solutions (e.g., fixing JuliaRegistries/Registrator.jl#122, which actually has a lot of overlap with the registry tests), but what you're proposing would catch a lot of issues.

I guess the main thing to think about is whether there is a way to centralize more of this so one doesn't have to update a configuration manually in so many packages. I worry about things changing over time and having to manually make a bunch of fixes. Is there any chance some of this could go in stdlib/Test and/or the default travis script (https://github.com/travis-ci/travis-build/blob/master/lib/travis/build/script/julia.rb)?

@timholy
Copy link
Member

timholy commented May 16, 2019

One other concern: failures due to the time gap between bumping the version in Project.toml and the merge of the registry PR. If you clone the master branch, you'll get the version bump but other packages with restrictive version bounds will not be loosened yet.

EDIT: one approach would be to fix those bounds locally. It would require a dive into Pkg internals, though.

In case it's not clear what I mean by "loosening" and why I think this could be a concern: the long-term hope for the General repository is that we use both lower & upper bounds on basically all dependencies. Specifying bounds as DependencyA = ">=0.2" is placing too much trust in the fact that all future releases of DependencyA will continue to work exactly as they did in 0.2. So specifying it as DependencyA = "0.2, 0.3, 0.4" certifies specific versions as working with this package; when a new 0.5 release gets made, it requires re-certification. This is going to change the release process quite substantially, and I think to make it viable we're going to need more automated testing tools.

For this issue, it's only a concern if someone (1) merges a version-incompatible bump in a dependent package, and then (2) someone else submits a PR to a consumer of that package in the intervening time before the PR to the General registry merges. While I expect (1) to become increasingly infrequent as packages stabilize, (2) can sometimes be a gap of days, especially if some tests fail and we need to wait for authors to fix problems.

@johnnychen94
Copy link
Member Author

johnnychen94 commented May 16, 2019

Yes you're right, and I'm more than convinced now 😄Your solution is way better than mine.

Close this issue and pending PRs 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

No branches or pull requests

2 participants