Travis configuration

[dunn]

The secure environment variables need to be refreshed:

• BT_TEST_MAIN_USER
• BT_TEST_MAIN_PASS
• CONSUMER_KEY
• CONSUMER_SECRET

I used the travis gem:

travis encrypt CONSUMER_SECRET=etcetc

and so on.

[dunn]

Ah, forgot all the new stuff needs to be in a if [ "$TRAVIS_PULL_REQUEST" = "false" ] block: https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions

[dunn]

Instead of doing big conditional blocks in Travis, I might pull the new steps into a shell script.

[dunn]

Not as pretty, but it works.

[jsha]

Thanks for writing this! I think TRAVIS_PULL_REQUEST isn't the right thing to check. For instance, I often use pull requests for my own code. Among other things, it's a handy way to make sure tests pass. Instead, I would check whether the credentials are available in env.

Also, I think we should run all the steps unconditionally, except the ones that will fail without credentials. For instance, npm install is fine, but npm test is expected to fail.

[jsha]

For this type of stuff, I usually put a cd $(dirname $0) at the top of the file. You could do the same with ${BASH_SOURCE[0]} (I assume that's more broadly accurate?).

[dunn]

I think I assumed $0 had a different value, but it looks like BASH_SOURCE is slightly more reliable: http://stackoverflow.com/a/179231

But I'll add the cd to the top. Should we also do set -e?

[jsha]

Yep, -e is good.

[dunn]

Thanks for writing this! I think TRAVIS_PULL_REQUEST isn't the right thing to check. For instance, I often use pull requests for my own code. Among other things, it's a handy way to make sure tests pass. Instead, I would check whether the credentials are available in env.

For PRs from branches that are part of your own repository, Travis will build them twice as long as "Build pushes" is turned on; once as a PR and once as a push. But I also thought encrypted variables weren't available for any PRs, when they are only hidden from PRs coming from forks. So I'll update to check for the variable(s).

[dunn]

Also, I think we should run all the steps unconditionally, except the ones that will fail without credentials. For instance, npm install is fine, but npm test is expected to fail.

What would be the benefit of running npm install?

[jsha]

What would be the benefit of running npm install?

It makes the .travis.yml simpler, so there are fewer places where a reader has to say "Why is there a conditional here?"

[dunn]

OK, fair enough.

[jsha]

Can you put a ./ in from of this and run-dev.sh? I generally like to do that in case . isn't in PATH, which is fairly common. I'm actually surprised it is in Travis' PATH.

[dunn]

Sweet, we're passing again: https://travis-ci.org/dunn/blocktogether/builds/157387268

[jsha]

https://docs.travis-ci.com/user/database-setup/#Starting-Services

"If you install a service in the addons: section, such as MariaDB, you do not need to add it to the services: section as well."

[jsha]

My understanding of Travis build lifecycle is that before_install and before_script are only useful if you want to rely on the default install or script steps that Travis provides for a given environment: https://docs.travis-ci.com/user/customizing-the-build#Customizing-the-Installation-Step.

Since we're overriding install and script, simpler to move these steps into those and reduce the number of sections we have. That should also allow you to merge before_install.sh and before_script.sh into a single script, say travis_setup.sh, that gets run in the install phase.

[dunn]

Oh, yeah! Will do.

[jsha]

Looks like you forgot to git add ./bin/travis/install.sh

[dunn]

🙀

[dunn]

Yeah, this is the error I saw before: https://travis-ci.org/dunn/blocktogether/jobs/157389915#L277

[jsha]

I think this is the cause:

https://stackoverflow.com/questions/26299552/travis-sudo-is-disabled

I'm guessing that when you put sudo in the .travis.yml, Travis figures it out and automatically disables the Docker-based infrastructure, but when it happens only in the shell script, Travis doesn't know, so the job runs on the Docker-based infrastructure where sudo is not enabled. Instead of including sudo in the Travis file, let's add:

sudo: true

to the config. I think we can do some refactoring to make things work without sudo, but that's work for another change.

[dunn]

Looks like ${CONSUMER_KEY} is returning true: https://travis-ci.org/jsha/blocktogether/jobs/157390801

[dunn]

🌲  "[ ${NONEXISTENT} ] && echo hello"
-bash: [  ] && echo hello: command not found

Oh, it's the quotes messing it up.

[jsha]

Yep - there are two layer of interpolation here, I think. The string gets evaluated once, interpolating ${NONEXISTENT} to be the empty string. Then that becomes what gets run, which doesn't have a token in that position.

Did you conclude that Travis' YAML parser really does need those quotes?

[dunn]

Did you conclude that Travis' YAML parser really does need those quotes?

Yeah, I tried without and it died with the same error message: https://travis-ci.org/dunn/blocktogether/builds/157388926

I got 1/2 passing this time: https://travis-ci.org/dunn/blocktogether/builds/157391226. I went back to having them run simultaneously, but I'm not sure why that would matter.

[jsha]

Oh right, I forgot! Ok, I'm convinced that the if;then approach works better here.

[dunn]

Re-ran the failing job and it passed.

[jsha]

Great. LGTM, will merge later tonight when I have a chance to set up the
env vars correctly. Thanks so much!

On Sat, Sep 3, 2016 at 10:28 PM, Alex Dunn notifications@github.com wrote:

Re-ran the failing job and it passed.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#229 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AANcLYcJaXQVOGoV7Ttx1SG1-5E2E8R3ks5qmizIgaJpZM4JyPYd
.

[jsha]

Per https://docs.travis-ci.com/user/environment-variables/:

if it does not contain sensitive information, might be different for different branches and should be available to forks – add it to your .travis.yml
if it does contain sensitive information, and might be different for different branches – encrypt it and add it to your .travis.yml
if it does contain sensitive information, but is the same for all branches – add it to your Repository Settings

The third case is true, so I'm going to set the vars in Repository Settings, then merge this, then remove the env vars lines from .travis.yml.

[dunn]

Great!