Deploy documentation and make extended tests visible #1447

Croydon · 2018-02-27T22:18:28Z

As discovered in #1402 the docs aren't getting deployed anymore.
Also the tests don't run.

This fixes both (fix #1402) and has some other simplifications.

The tests are failing right now. This is already the case in the current master branch, but was hidden since no tests ran...

1 is too low and causes already running builds to fail if you push new commits, there are no good reasons for this confusing behaviour

JimiC · 2018-02-28T09:29:02Z

@Croydon I see you are switching to stages. Great! I was going to propose that. The only objection I have to this PR is that documentation should be deployed after all test pass, not before.
I'll try to propose a travis configuration within the day.

Croydon · 2018-02-28T12:17:23Z

Before the documentation gets deployed all tests already ran.

If you put it at the end, you first deploy new binaries and then documentation, which is worse in my opinion.

JimiC · 2018-02-28T12:21:20Z

You see the point with stages is to run your test first and when they pass you move to the stage where you deploy the binaries and then to the stage where you update the docs. At least that's the steps I take in other projects and that's the one I'll propose.

Croydon · 2018-02-28T12:32:07Z

This is possible as well, sure.

Current order in this pull request:

ran all implemented tests
if former step succeeded: deploy documentation
if former step succeeded: build and deploy all binaries

Order in master right now:

build and deploy binaries if some smaller tests are fine
SOMEWHEN ran more extensive tests (when NODE VERSION = 6)
- this didn't worked anymore, since no such versions existed in the CI configuration
- even if this version would exist, it would hide the extensive testing "somewhere" within the amount of jobs, without visible indiciation, and even if the extensive tests would fail, all other binaries would get deployed

So this pull request is already a huge improvement to this. You can surely improve it even more, but I think that would be follow up work.

What is missing in this pull request is possibly the fixing of the tests, however, technically speaking even this is another issue.

JimiC · 2018-02-28T12:55:53Z

From what I see, currently travis does:

Setup
Run test
- if tagged and OS is linux and node version is 6 then run test and produce coverage reports and send them to coveralls
  -If tagged then deploy binaries
if branch is master and not a PR and OS is linux and node version is 6 and targeted arch os x64 then deploy documentation.

With stages we can have:

Setup
Stage Test (if conditions are met)
- Run tests
Stage Code Coverage report (if condition are met)
- Run tests and run tests on compiled code and generate report and upload
Stage Deploy Binaries (if condition are met)
- Compile and deploy binaries (no need to runt tests here again)
Stage Deploy Documentation (if condition are met)
- Compile and run deploy-docs script (no need to run tests here again)

Note here that we can trigger 'Stage Test' on every occasion except release and all others on release.
Also, the stage order should be as described.

JimiC · 2018-02-28T13:01:11Z

Come to think of it 'Stage Code Coverage report' can be combined with 'Stage Test' by altering the running travis script.

See issue nodegit#1449

JimiC · 2018-02-28T13:15:36Z

@Croydon What does the before_script do?

JimiC · 2018-02-28T14:18:46Z

sudo: false

compiler: clang

language: node_js

os:
  - linux
  - osx

node_js:
  - node
  - 8
  - 7
  - 6

env:
  matrix:
    - TARGET_ARCH="x64"
    - TARGET_ARCH="ia32"

matrix:
  fast_finish: true
  exclude:
    - os: osx
      env: TARGET_ARCH="ia32"

git:
  depth: 1

addons:
  apt:
    sources:
      - ubuntu-toolchain-r-test
    packages:
      - build-essential
      - libssl-dev
      - gcc-4.9-multilib
      - g++-4.9-multilib
      - lcov

before_install:
  - export CC=clang
  - export CXX=clang++
  - export npm_config_clang=1
  - export JOBS=4
  - export GYP_DEFINES="use_obsolete_asm=true"

# This is a random private key used purely for testing.
before_script:
  - echo -e "Host *\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config
  - echo -e "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDkTcgXnHuqR0gbwegnr9Zxz4hTkjjV/SpgJNPJz7mo/HKNbx0rqjj1P0yGR053R9GSFFim2ut4NK9DPPUkQdyucw+DoLkYRHJmlJ4BNa9NTCD0sl+eSXO2969kZojCYSOgbmkCJx8mdgTwhzdgE/jhBrsY0hPE6pRTlU+H68/zeNdJUAIJf0LLXOm3hpTKLA19VICltl/j9VvBJpgRHdBylXEyL8HokYpjkQQk1ZXj3m7Nlo8yDdg4VcljOJWC+Xh8kxRMfK5x/VRVsYKCQXN5QlzKeqf7USRDUS/7mFoPUBW+d4kwKtGxRsWuIL2yeqzifZUTOgsh9+ZWAWxWffQZ your_email@example.com" > ~/.ssh/id_rsa.pub
  - echo -e "-----BEGIN RSA PRIVATE KEY-----\nMIIEpAIBAAKCAQEA5E3IF5x7qkdIG8HoJ6/Wcc+IU5I41f0qYCTTyc+5qPxyjW8d\nK6o49T9MhkdOd0fRkhRYptrreDSvQzz1JEHcrnMPg6C5GERyZpSeATWvTUwg9LJf\nnklztvevZGaIwmEjoG5pAicfJnYE8Ic3YBP44Qa7GNITxOqUU5VPh+vP83jXSVAC\nCX9Cy1zpt4aUyiwNfVSApbZf4/VbwSaYER3QcpVxMi/B6JGKY5EEJNWV495uzZaP\nMg3YOFXJYziVgvl4fJMUTHyucf1UVbGCgkFzeUJcynqn+1EkQ1Ev+5haD1AVvneJ\nMCrRsUbFriC9snqs4n2VEzoLIffmVgFsVn30GQIDAQABAoIBAQDPQm2sQbti0mN8\nD4Uawl8D40v30n8WhUa7EbPTOmlqKAQ2sfDhex9KRbTLEmEBmImA/Eee8o9iCTIy\n8Fv8Fm6pUHt9G6Pti/XvemwW3Q3QNpSUkHqN0FDkgecQVqVBEb6uHo3mDm4RFINX\neOmkp30BjIK9/blEw1D0sFALLOEUPaDdPMwiXtFgqfrFSgpDET3TvQIwZ2LxxTm0\ncNmP3sCSlZHJNkZI4hBEWaaXR+V5/+C1qblDCo5blAWTcX3UzqrwUUJgFi6VnBuh\n7S9Q6+CEIU+4JRyWQNmY8YgZFaAp6IOr/kyfPxTP1+UEVVgcLn3WDYwfG9og0tmz\nfzlruAgBAoGBAPfz73Pey86tNZEanhJhbX8gVjzy2hvyhT0paHg0q/H6c1VWOtUH\nOwZ3Ns2xAZqJhlDqCHnQYSCZDly042U/theP4N8zo1APb4Yg4qdmXF9QE1+2M03r\nkS6138gU/CSCLf8pCYa6pA/GmsaXxloeJGLvT4fzOZRsVav80/92XHRhAoGBAOu2\nmKh4Gr1EjgN9QNbk9cQTSFDtlBEqO/0pTepvL73UvNp/BAn4iYZFU4WnklFVBSWc\nL84Sc732xU12TAbTTUsa6E7W29pS8u7zVTxlIdQIIU5pzDyU1pNNk2kpxzte5p3Y\nPDtniPFsoYLWoH0LpsKL93t2pLAj+IOkE6f3XBq5AoGAIKaYo5N1FxQr952frx/x\nQUpK0N/R5Ng8v18SiLG26rhmM5iVSrQXC7TrHI7wfR8a9tC6qP/NqnM9NuwC/bQ0\nEEo7/GhaWxKNRwZRkmWiSFLNGk9t1hbtGU+N1lUdFtmloPIQdRNiw0kN3JTj474Q\nYI7O1EItFORnK6yxZfR6HEECgYEA1CT7MGUoa8APsMRCXyaiq15Pb8bjxK8mXquW\nHLEFXuzhLCW1FORDoj0y9s/iuKC0iS0ROX8R/J7k5NrbgikbH8WP36UxKkYNr1IC\nHOFImPTYRSKjVsL+fIUNb1DSp3S6SsYbL7v3XJJQqtlQiDq8U8x1aQFXJ9C4EoLR\nzhKrKsECgYBtU/TSF/TATZY5XtrN9O+HX1Fbz70Ci8XgvioheVI2fezOcXPRzDcC\nOYPaCMNKA5E8gHdg4s0TN7uDvKTJ+KhSg2V7gZ39A28dHrJaRX7Nz4k6t2uEBjX9\na1JidpAIbJ+3w7+hj6L299tVZvS+Y/6Dz/uuEQGXfJg/l/5CCvQPsA==\n-----END RSA PRIVATE KEY-----" > ~/.ssh/id_rsa
  - chmod 600 ~/.ssh/id_rsa*
  - eval `ssh-agent -s`
  - ssh-add ~/.ssh/id_rsa
  - git config --global user.name "John Doe"
  - git config --global user.email johndoe@example.com

script: travis_retry npm test

jobs:
  include:
    - stage: Code Coverage Report
      if: repo =~ ^nodegit AND tag IS present AND type != pull_request
      os: linux
      node_js: 6
      before_install:
        - export GYP_DEFINES="coverage=1 use_obsolete_asm=true";
        - export CC=/usr/bin/gcc-4.9;
        - export CXX=/usr/bin/g++-4.9;
        - export npm_config_clang=0;
        - wget http://downloads.sourceforge.net/ltp/lcov-1.10.tar.gz;
        - tar xvfz lcov-1.10.tar.gz;
      before_script:
      script: travis_retry npm test && npm run cov && npm run coveralls

    - stage: Deploy Binaries
      if: repo =~ ^nodegit AND tag IS present AND type != pull_request
      os: linux
      node_js: 6
      env:
        - TARGET_ARCH="x64"
        - TARGET_ARCH="ia32"
      before_script:
      script:
        - npm install -g node-pre-gyp;
        - npm install -g aws-sdk;
        - node lifecycleScripts/clean;
        - node-pre-gyp package --target_arch=$TARGET_ARCH;
        - node-pre-gyp publish --target_arch=$TARGET_ARCH;
    
    - stage: Deploy Documentation
      if: repo =~ ^nodegit AND tag IS present AND type != pull_request
      os: linux
      node_js: 6
      before_script:
      script: bash .travis/deploy-docs.sh      

notifications:
  slack:
    secure: KglNSqZiid9YudCwkPFDh+sZfW5BwFlM70y67E4peHwwlbbV1sSBPHcs74ZHP/lqgEZ4hMv4N2NI58oYFD5/1a+tKIQP1TkdIMuq4j2LXheuirA2HDcydOVrsC8kRx5XFGKdVRg/uyX2dlRHcOWFhxrS6yc6IxtxYWlRTD2SmEc=

  webhooks:
    urls:
      - https://webhooks.gitter.im/e/cbafdb27ad32ba746a73
    on_success: always  # options: [always|never|change] default: always
    on_failure: always  # options: [always|never|change] default: always
    on_start: false     # default: false

Here is my proposal of a simplified travis config. I still don't know what before_script does so you may have to adjust accordingly If you decide to adopt it.

Croydon · 2018-02-28T19:04:53Z

@JimiC I haven't used stage conditions or grouping the script steps into the stages on purposes as it is too buggy right now. See travis-ci/beta-features#28

Beyond that, your script seems to do the same as my version.

before_script is preparing the environment for the steps later. Like setting some flags for GYP and downloading lcov.

From my perspective this pull request's work is done. Not sure who needs to review this. @implausible maybe?

JimiC · 2018-02-28T19:41:31Z

What you describe is before_install. before_script looks like some setup for git but as I'm not familiar with the entire project, I'm hesitant to remove it.

Regarding the condition declaration in stages what I used is safe as I have dealt with the bugs in another project and used those that work.

I'm merely proposing simplicity versus complexity for the travis config. I believe that my proposal increases readability and maintainability. But I'm just passing through here... so no biggy.

implausible · 2018-03-05T18:28:47Z

@Croydon I will get to this before the next release so we can start having good docs again.

implausible · 2018-03-16T21:07:11Z

This was cancelled/failing on travis. @Croydon

Croydon · 2018-03-16T21:09:36Z

@implausible It's weird. Either the stage condition did work in this case (it's still a buggy beta feature), but then the overall build status should not be set to "failed". And if there was another reason then there should be some log...

Could you please just restart the build?

Croydon · 2018-03-16T22:43:00Z

I will have a look in the next 36 hours.

implausible · 2018-03-16T22:43:22Z

@Croydon I canceled the PR this time. It looks like you need another condition in the deploy stage which only causes it to run if the build is NOT in a PR. It is trying to update documentation in this PR.

Croydon · 2018-03-16T22:44:49Z

No, let it run.

The job runs but the documentation will NOT get deployed. https://github.com/inexorgame/nodegit/blob/f725ac5950b0ba7fb126c0845212f1a78377a62e/.travis.yml#L106

implausible · 2018-03-16T22:45:44Z

Aha, thanks that's what I was missing. Thanks @Croydon!

implausible · 2018-03-16T22:48:31Z

.travis.yml

@@ -98,7 +103,7 @@ after_success:
      node-pre-gyp publish --target_arch=$TARGET_ARCH;
    fi

-  - if [ $TRAVIS_BRANCH == "master" ] && [ $TRAVIS_PULL_REQUEST == "false" ] && [ $TRAVIS_OS_NAME == "linux" ] && [ $NODE_VERSION == "6" ] && [ $TARGET_ARCH == "x64" ]; then
+  - if [ $TRAVIS_BRANCH == "master" ] && [ $TRAVIS_PULL_REQUEST == "false" ] && [ $DEPLOY_DOCUMENTATION == "true" ]; then


We should not deploy documentation unless it is a deploy build. Please add the TRAVIS_TAG condition.

implausible · 2018-03-19T16:26:34Z

Thanks @Croydon!

Croydon added 5 commits February 27, 2018 18:50

Travis: Fix coveralls execution and documentation deploy

0603658

Travis: Simplify Node installing and version matrix

78deec2

Travis: Give extended testing and doc deploying an own stage

f471a22

Travis: Increase depth of cloning

6ccc46b

1 is too low and causes already running builds to fail if you push new commits, there are no good reasons for this confusing behaviour

Travis: Always use latest minor release of major

aa24124

Croydon mentioned this pull request Feb 27, 2018

Nodegit compiles only for very few configurations inexorgame-obsolete/deprecated-cube-engine-inexor#515

Closed

Travis: Split extended testing and documentation deploy stage

9d874f5

Croydon mentioned this pull request Feb 28, 2018

Fix code coverage of tests #1449

Open

Travis: Disable tests for now

c2e8838

See issue nodegit#1449

Croydon changed the title ~~Deploy documentation and run tests~~ Deploy documentation and make extended tests visible Feb 28, 2018

Drop Node.js 7 (fix nodegit#1446)

f725ac5

implausible requested changes Mar 16, 2018

View reviewed changes

Travis: Only deploy documentation on tags

f25988c

implausible approved these changes Mar 19, 2018

View reviewed changes

implausible merged commit 204d621 into nodegit:master Mar 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deploy documentation and make extended tests visible #1447

Deploy documentation and make extended tests visible #1447

Croydon commented Feb 27, 2018 •

edited

JimiC commented Feb 28, 2018

Croydon commented Feb 28, 2018

JimiC commented Feb 28, 2018 •

edited

Croydon commented Feb 28, 2018

JimiC commented Feb 28, 2018 •

edited

JimiC commented Feb 28, 2018

JimiC commented Feb 28, 2018

JimiC commented Feb 28, 2018

Croydon commented Feb 28, 2018

JimiC commented Feb 28, 2018

implausible commented Mar 5, 2018

implausible commented Mar 16, 2018

Croydon commented Mar 16, 2018 •

edited

Croydon commented Mar 16, 2018

implausible commented Mar 16, 2018

Croydon commented Mar 16, 2018

implausible commented Mar 16, 2018

implausible Mar 16, 2018

implausible commented Mar 19, 2018

Deploy documentation and make extended tests visible #1447

Deploy documentation and make extended tests visible #1447

Conversation

Croydon commented Feb 27, 2018 • edited

JimiC commented Feb 28, 2018

Croydon commented Feb 28, 2018

JimiC commented Feb 28, 2018 • edited

Croydon commented Feb 28, 2018

JimiC commented Feb 28, 2018 • edited

JimiC commented Feb 28, 2018

JimiC commented Feb 28, 2018

JimiC commented Feb 28, 2018

Croydon commented Feb 28, 2018

JimiC commented Feb 28, 2018

implausible commented Mar 5, 2018

implausible commented Mar 16, 2018

Croydon commented Mar 16, 2018 • edited

Croydon commented Mar 16, 2018

implausible commented Mar 16, 2018

Croydon commented Mar 16, 2018

implausible commented Mar 16, 2018

implausible Mar 16, 2018

Choose a reason for hiding this comment

implausible commented Mar 19, 2018

Croydon commented Feb 27, 2018 •

edited

JimiC commented Feb 28, 2018 •

edited

JimiC commented Feb 28, 2018 •

edited

Croydon commented Mar 16, 2018 •

edited