Add verify stage to TravisCI config

[citizen428]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update

Description

As discussed on Slack, this PR adds a verify stage to the Travis pipeline which will try to start a production Rails runner to verify that the app can boot up, so we don't solely rely on deploy-tasks.sh for that purpose.

Related Tickets & Documents

n/a

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

n/a

Added to documentation?

• docs.dev.to
• readme
• no documentation needed

[citizen428]

@mstruve I guess for this to work we'd need to set up a few env variables, most likely to dummy values.

[citizen428]

Another problem is that after_scripts return code does not influence the build status 😢

The exit code of after_success, after_failure, after_script, after_deploy and subsequent stages do not affect the build result. However, if one of these stages times out, the build is marked as failed.

I'll see what else I can come up with, but we may need to move the config from simple lifecycle to actual stages.

[mstruve]

Why not just add it as another step to the initial script so if it fails the build fails?

Hopefully, setting those dummy values aren't a rabbit hole 🤞

[citizen428]

Why not just add it as another step to the initial script so if it fails the build fails?

I was thinking about this, but it just felt like a distinct stage to me (as does storybook updating tbh). But in lieu of stages, this is probably the only sensible approach without completely changing our Travis settings.

[maestromac]

@citizen428 random suggestion: would
RAILS_ENV=production bundle exec rails runner 'puts "App booted successfully" || travis_terminate 1 work here?

[citizen428]

@maestromac It shouldn't be necessary, as the exit code of script determines the build status. So if the Rails runner fails, the build should already fail.

[citizen428]

@mstruve I think the amount of dummy env variables needed to get the Rails runner to start up is manageable.

[citizen428]

Without this I always got a uninitialized constant ApplicationConfig (NameError) on Travis and locally. I think it makes sense to keep this consistent with line 103 anyway.

[citizen428]

@mstruve This is ready for review now.

[mstruve]

This works, one downside is if we remove this from the release-tasks.sh script it does move us further away from actual production because we are not loading the environment with the actual production ENV variables. If a variable we stub here was misconfigured in production this would pass but production would go down. What do you think about keeping both for now approaches for now?

[vaidehijoshi]

I think it would be nice to document that these are env variables only required for the verify stage on Travis. I can see someone looking at this down the road and wondering why we have these or whether it
s safe to change or remove them.

Can we add a comment in the travis.yml file for our future selves?

[citizen428]

Can and will, was just waiting to see if we even go ahead with this approach at all. 😃 Thanks for the reminder though! 👍

[vaidehijoshi]

Makes sense! And thanks :)

[citizen428]

@mstruve Agreed, my intention was not to remove it from deploy-tasks.sh, but to catch certain cases pre-deployment. Sorry if I didn't communicate this better, admittedly

so we don't solely rely on deploy-tasks.sh for that purpose.

wasn't the most explicit.

[nickytonline]

LGTM , but as @vaidehijoshi mentioned, it'd be good to document those vars.

[benhalpern]

This works for me. I'll let @mstruve merge this unless you have last second cold feet on it.