Spec Fixes + Rails 5 Rough Edges #388

toastercup · 2016-11-18T02:21:43Z

This is a very large PR (with many side-effect changes) with a few significant changes, so I'm fine debating and letting this one sit for a bit. Namely, the Rails 5 defaults, the rebuilt test harness (the documentation from rspec/rspec-rails is light on how to require these files properly), and the ES changes are all things I want to do more research on before a merge. Either way, this gets specs passing on Semaphore again! Details:

Upgrade Elasticsearch, remove unnecessary extensions and config options, ensure seeding doesn't happen twice, add ONET and Custom Content seeding, re-arrange bower setup, install bower devDependencies for test running, tweak sleep time
Create Rails 5 ApplicationRecord abstract class, use dotenv-rails to load environment variables even earlier, use Rails 5-compatible awesome_nested_set, add missing User/has_many/Posts association, remove old Mongo-related code, switch new_framework_defaults to Rails 5 defaults (an optimistic decision; potentially premature), eager_load Rails for test environment in environment config rather than test setup
Use Redis as the session_store, which should eliminate cookie errors we're seeing in New Relic on Production, and potentially speed things up, as well
- New SESSION_STORE_URL env variable now required
Add missing_indices migration from acts_as_taggable_on
Fix broken migrations by ensuring FactoryGirl lazy-loads hardcoded class references; reference new cortexcms.org domain; utilize recommended ApplicationJob abstract class; remove unused tenantUsers scope; remove client_skips_authorization? assumptions - no longer needed; set AR generators to use UUID PKs by default
Fix API Documents spec
Fix a bunch of specs by using FactoryGirl's build strategy instead of create, which doesn't persist factory to AR/ES; should speed up specs; these UserAbility specs aren't testing persistence, just authorization, so this is reasonable
Mark a couple tests pending and document why
Use rspec gem to execute specs instead of rake task, per their documentation, for semaphore; this actually is running many more spec files now
Fix up spec_helper require statements; fix Simplecov coverage report generation
Completely refactor test harness, add @ElliottAYoung's coverage improvements
Add .rspec for pretty colors!
Switch to HTTPS git URIs, per bundler warnings
Fix ES on Semaphore by letting ES start and stop itself
Implement MDL Flash functionality
Switch to jquery v3

…ns, ensure seeding doesn't happen twice, add ONET and Custom Content seeding, re-arrange bower setup, install bower devDependencies for test running, tweak sleep time

…load environment variables even earlier, use Rails 5-compatible awesome_nested_set, add missing User/has_many/Posts association, remove old Mongo-related code, switch new_framework_defaults to Rails 5 defaults (an optimistic decision; potentially premature), eager_load Rails for test environment in environment config rather than test setup

…ass references; reference new cortexcms.org domain; utilize recommended ApplicationJob abstract class; remove unused tenantUsers scope; remove client_skips_authorization? assumptions - no longer needed; set AR generators to use UUID PKs by default

…which doesn't persist factory to AR/ES; should speed up specs; these UserAbility specs aren't testing persistence, just authorization, so this is reasonable

…ort generation

…, fix coverage generation

Casao · 2016-11-18T13:46:30Z

.rspec

@@ -0,0 +1,2 @@
+--color
+--require spec_helper


I'd suggest being explicit in the files requiring spec helper instead of this.

👍 I was unsure about this - the docs conflicted themselves in a couple spots. Moving this out of .rspec.

I'm not sure there's any real NEED either way. But I feel .rspec is better for personal configurations imo.

Blah.. upon looking into it more, getting rid of this means we have to have require statements one very one of our specs. I'd rather favor a global configuration like .rspec to handle this tedium..

Casao · 2016-11-18T13:50:48Z

config/environments/test.rb

@@ -11,7 +10,7 @@
  # Do not eager load code on boot. This avoids loading your whole application
  # just for the purpose of running a single test. If you are using a tool that
  # preloads Rails for running tests, you may have to set it to true.
-  config.eager_load = false
+  config.eager_load = true


As the app gets bigger, this is gonna really slow down your specs, making it harder to run them locally.

This is part of a larger attempt at making coverage more accurate across our applications - Elliott could speak more to this, but we're eager loading so that Simplecov is aware of all the LOC of our application. Without this, it misses a few files and gives a higher coverage %.

Here's the relevant PR from Employer: https://github.com/cbdr/employer/pull/551
I'm waiting to pull some of this functionality in to fix up some namespacing and turn some stuff into modules.

To address the concern - yeah, it's definitely going to slow this down. My only thought is we could re-introduce Spring.

Ah.

We just solved this by having good code coverage 😆

Casao · 2016-11-18T13:54:57Z

spec/spec_helper.rb

-Capybara.register_driver :poltergeist do |app|
-  Capybara::Poltergeist::Driver.new(app, js_errors: false, timeout: 900.seconds)
-end
+require 'rails_helper'


rails helper is requiring spec helper already, so this is circular. Don't think it hurts for requires, but you might as well merge them into one file in that case.

The general advice I've seen is rails_helper should be for feature specs/rspec-rails specs, while the spec_helper is for independent classes (services, interactors, anything that doesn't depend on the rails stack).

👍 fixing the require statements here. I think I've mostly followed that pattern; Rails-y things are in the rails_helper. I think there's a couple things I could tighten up in spec_helper, though. Thanks for your help and second pair of eyes!

ElliottAYoung · 2016-11-22T16:20:44Z

.env.example

-SMTP_SENDER_DOMAIN=cbcortex.com
-SMTP_SENDER_ADDRESS=noreply@cbcortex.com
+SMTP_SENDER_DOMAIN=cortexcms.org
+SMTP_SENDER_ADDRESS=noreply@cortexcms.org


Do we own cortexcms.org?

Ayep! See: docs.cortexcms.org

ElliottAYoung · 2016-11-22T16:25:36Z

README.md

@@ -144,7 +144,7 @@ $ npm install -g bower && bundle exec rake bower:install:development
 * Create databases:

 ```sh
-bundle exec rake db:create:all
+$ bundle exec rake db:create


This is a personal preference more than anything, but could we not have "$" at the start of the command lines in the README? It means I have to drag copy rather than double click...and you know...saving precious seconds and all

I was going to agree with you, but I dug into the top trending/open source repos on Github, and they all follow the $ convention. This is because occasionally, one may want to include the stdout output in the highlighted code block, and $ makes clear that you're going to be typing that expression as input into a shell prompt that expects it. I still want to dig into this more and see who avoids $'s, and maybe poke some people on IRC about it. Thanks for the suggestion, though - I certainly get annoyed by it when I'm copying/pasting commands from a readme.

Ugh - life is hard and the struggle is real

ElliottAYoung

I've nitpicked and mourned the loss of the Marquee, but it looks good to me 👍

…w required

…ec_helper

toastercup added 13 commits November 14, 2016 19:54

Upgrade Elasticsearch, remove unnecessary extensions and config optio…

81fe4cc

…ns, ensure seeding doesn't happen twice, add ONET and Custom Content seeding, re-arrange bower setup, install bower devDependencies for test running, tweak sleep time

Add missing_indices migration from acts_as_taggable_on

ee9a881

Fix API Documents spec

f28c7e7

Fix a bunch of specs by using FactoryGirl's build instead of create, …

b84edd8

…which doesn't persist factory to AR/ES; should speed up specs; these UserAbility specs aren't testing persistence, just authorization, so this is reasonable

Mark a couple tests pending and document why

31956b2

Don't use rspec task for semaphore

adfa38b

Fix up spec_helper requires; attempting to fix Simplecov coverage rep…

7157098

…ort generation

Completely refactor test harness, add Elliott's coverage improvements…

29cf846

…, fix coverage generation

Add .rspec

a2876da

Switch to HTTPS git URIs

e6e730e

Attempt to fix ES spec hook

65f9583

toastercup added bug enhancement infrastructure technical debt labels Nov 18, 2016

toastercup added this to the Open Source milestone Nov 18, 2016

toastercup force-pushed the semaphore-build-fixes branch from 801317d to b91ec42 Compare November 18, 2016 02:33

Let ES start and stop itself

610c96f

toastercup force-pushed the semaphore-build-fixes branch from b91ec42 to 610c96f Compare November 18, 2016 02:48

Switch to jQuery 3, add MDL Flash functionality, rearrange gemfile..

ab90ed0

Casao reviewed Nov 18, 2016

View reviewed changes

ElliottAYoung reviewed Nov 22, 2016

View reviewed changes

ElliottAYoung approved these changes Nov 22, 2016

View reviewed changes

toastercup added 2 commits November 22, 2016 10:46

Use Redis as the session_store. New SESSION_STORE_URL env variable no…

9fed96d

…w required

Remove circular require statements, remove unnecessary method from sp…

be92af2

…ec_helper

This was referenced Nov 22, 2016

ElasticSearch after suite is breaking Semaphore #361

Closed

Rails 5 #291

Closed

toastercup merged commit f91181e into develop Nov 22, 2016

toastercup deleted the semaphore-build-fixes branch November 22, 2016 17:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spec Fixes + Rails 5 Rough Edges #388

Spec Fixes + Rails 5 Rough Edges #388

toastercup commented Nov 18, 2016 •

edited

Loading

Casao Nov 18, 2016

toastercup Nov 18, 2016

Casao Nov 18, 2016

toastercup Nov 22, 2016

Casao Nov 18, 2016

toastercup Nov 18, 2016

toastercup Nov 18, 2016

Casao Nov 18, 2016

Casao Nov 18, 2016

toastercup Nov 18, 2016

ElliottAYoung Nov 22, 2016

toastercup Nov 22, 2016

ElliottAYoung Nov 22, 2016

toastercup Nov 22, 2016

ElliottAYoung Nov 22, 2016

ElliottAYoung left a comment

Spec Fixes + Rails 5 Rough Edges #388

Spec Fixes + Rails 5 Rough Edges #388

Conversation

toastercup commented Nov 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElliottAYoung left a comment

Choose a reason for hiding this comment

toastercup commented Nov 18, 2016 •

edited

Loading