Rado's blog

Using "data-test" in testing

When testing HTML components, I often see people using class names as selectors. For example:

element.find('.description button.expand-button').simulate('click');  

While this seems convenient at first, there are some drawbacks. HTML structure and css classes tend to change due to design changes. Which will cause you re-write tests quite often. Also, if you are using css-modules you can't rely on class names.

Because of that, for quite some time now, I have started marking elements with data-test attribute.

React example (using enzyme and chai-enzyme):

describe(Description.name, () => {  
  it('cut off text based on `cutoffLength`', () => {
    const el = shallow(<Description text="test" cutoffLength={1} />);

    expect(el).to.have.text('t...');
    expect(el).not.to.have.text('test');
  });

  it('hides expand button when text is short', () => {
    const el = shallow(<Description text="test" cutoffLength={10} />);
    expect(el).not.to.have.descendants('[data-text="expand-button"]');
  });

  it('shows expand button when text is long', () => {
    const el = shallow(<Description text="test" cutoffLength={1} />);
    expect(el).to.have.descendants('[data-test="expand-button"]');
  });

  it('clicking expand button reveals the whole text', () => {
    const el = shallow(<Description text="test" cutoffLength={1} />);

    el.find('[data-test="expand-button"]').simulate('click');

    expect(el).not.to.have.descendants('[data-test="expand-button"]');
    expect(el).to.have.text('test');
  });
});

The component code:

import React from 'react';  
import styles from "./style.css";

export default Description extends React.Component {  
  state = { expanded: false };

  render() {
    const { text, cutoffLength } = this.props;

    if (this.state.expanded || text.length < cutoffLength) {
      return (
        <div className={styles.description}>
          {this.props.text}
        </div>
      );
    }

    return (
      <div className={styles.description}>
        {`${ text.substr(0, cutoffLength) }...`}
        <button 
          data-test="expand-button" 
          className={styles.expand} 
          onClick={this.expand}>show more</button>
      </div>
    );
  }

  expand = () => {
    this.setState({ expanded: true });
  };
}

I'm also using data-test attributes for testing with Capybara in Ruby land.

describe 'Product page' do  
  it 'has product description rev' do
    product = create :post, :with_long_description

    visit product_path(product)

    expect(page).not_to have_text product.description

    # This can be extracted into `find_test_attr` or `click_test_attr`
    find(:css, '[data-test="expand"]').click

    expect(page).to have_text product.description

    # This can be extracted into `have_test_arr`
    expect(page).not_to have_css('[data-test="expand"]')
  end
end  

Retrying ActiveJob

I like ActiveJob. The one missing feature is the ability to retry when a job fails. Fortunately, this is quite easy to add:

module ActiveJobRetriesCount  
  extend ActiveSupport::Concern

  included do
    attr_accessor :retries_count
  end

  def initialize(*arguments)
    super
    @retries_count ||= 0
  end

  def deserialize(job_data)
    super
    @retries_count = job_data['retries_count'] || 0
  end

  def serialize
    super.merge('retries_count' => retries_count || 0)
  end

  def retry_job(options)
    @retries_count = (retries_count || 0) + 1
    super(options)
  end
end  
class ImageDownloader::Worker < ActiveJob::Base  
  include ActiveJobRetriesCount

  rescue_from ImageDownloader::TimeoutError do |exception|
    if exception.code == 0
      # just retry the download in 5 minutes
      # tolarate up to 5 failures
      retry_job wait: 5.minutes if retries_count > 5
    else
      fail exception
    end
  end

  def perform(url)
    ImageDownloader.download(url)
  end
end  

Here is a simple test for the ActiveJobRetriesCount.

require 'spec_helper'

describe ActiveJobRetriesCount do  
  class RetriesTestClass
  end

  class RetriesTestError < StandardError
  end

  class RetriesTestWorker < ActiveJob::Base
    include ActiveJobRetriesCount

    rescue_from RetriesTestError do
      retry_job wait: 5.minutes if retries_count < 5
    end

    def perform
      RetriesTestClass.do_something(retries_count)
    end
  end

  it 'handles retries counts', active_job: :inline do
    allow(RetriesTestClass).to receive(:do_something).and_raise RetriesTestError

    expect { RetriesTestWorker.perform_later }.not_to raise_error

    expect(RetriesTestClass).to have_received(:do_something).with(5)
    expect(RetriesTestClass).not_to have_received(:do_something).with(6)
  end
end  

If you are not on Rails 5, you would need the following code:

if ActiveJob::Base.method_defined?(:deserialize)  
  fail 'This no longer needed.'
else  
  module ActiveJob
    class Base
      def self.deserialize(job_data)
        job = job_data['job_class'].constantize.new
        job.deserialize(job_data)
        job
      end

      def deserialize(job_data)
        self.job_id               = job_data['job_id']
        self.queue_name           = job_data['queue_name']
        self.serialized_arguments = job_data['arguments']
      end
    end
  end
end  

For more advanced features – check out ActiveJob::Retry.

Custom routes in Rails

In the life of almost all Rails applications, comes the moment where custom routes are needed. Usually, the custom routes are just convenience methods like these:

def post_path(post)  
  date_post_path post.date_slug, post.slug
end

def product_url  
  date_post_url post.date_slug, post.slug
end  

In this way, you decouple the route generation from your objects. Also, if you are doing some system route update and you want to keep the old routes around - this is the way.

Usually, I create a module CustomPaths and put those methods there. But in recent years, Rails.application.routes.url_helpers is showing up in more and more places like - background jobs, presenters, serializers and so. This complicates things.

At Product Hunt we needed some custom paths. So, me and Mike Coutermarsh created the following object:

# => routes.rb
module Routes  
  # Simple `extend self` won't work here,
  # due to the way Rails implement `url_helpers`
  class << self
    include Rails.application.routes.url_helpers
    include Routes::CustomPaths
  end

  include Rails.application.routes.url_helpers
  include Routes::CustomPaths

  def default_url_options
    Rails.application.routes.default_url_options
  end
end

# => routes/custom_paths.rb
module Routes  
  module CustomPaths
    # ... all your custom route methods
  end
end  

It behaves the same way as Rails.application.routes.url_helpers plus the custom routes.

# in can be called directly
Routes.root_path  
Routes.product_path(product)

# or included in another object
class ObjectWhoNeedsRoutes  
  include Routes
end

ObjectWhoNeedsRoutes.new.root_path  
ObjectWhoNeedsRoutes.new.product_path  

I added Routes to my toolbox with install instructions and tests.

Rails Anti-Pattern: Setting View Variables In Before Actions

I see a lot of the following pattern in Rails projects:

class ProductsController < ApplicationController  
  before_action :assign_post, only: %i(show edit)

  def index
    @posts = Post.paginate(params[:page])
  end

  def show
  end

  def edit
  end

  private

  def assign_post
    @post = Post.find(params[:id])
  end
end  

Most people do this, because for them @post = Post.find(params[:id]) is duplication. Having assign_post is more DRY.

I don't buy the duplication argument. The definition for DRY is:

“Every piece of knowledge must have a single, unambiguous, authoritative representation within a system."

Unfortunately, most people confused it with - "I don't want to have the same sequence of characters more than once". David Chelimsky have an excellent talk on that subject.

But, my biggest issue with the approach is that it "hides" all variables passed to the view. Especially if there are only or except passed to before_action. Then you have to become an interpreter to figure out what gets into the view. I can recall several situations when during a code review I found code passing unwanted variables to a view.

I prefer to think the following, when I read a controller:

ok, show sends @post and @comments to the view

Instead of:

So, if show is called then assign_posts is called before it, which sets @posts, then show is not included in this other action filter, but is not excluded for this third one...

Also, it is easier for a developer to spot, that too many variables are passed to the view if all of them are listed together.

Because of that, I prefer to use the following pattern:

class ProductsController < ApplicationController  
  def show
    @post = find_post
  end

  def edit
    @post = find_post
  end

  private

  def find_post
    Post.find(params[:id])
  end
end  

In this way, the duplication is moved into finder methods. So if I have to add a scope like visible to Post.find, I just need to apply it in find_post.

Introducing MiniForm

My last blog post - Dealing with form objects, got me thinking about form objects. I also had a 12 hours flight to San Francisco, where I was meeting my teammates at Product Hunt.

During that flight, I went back and checked all of my custom form implementations. I gathered them together in a folder and started combining and extracting the common parts. By the end of the flight I almost had what now is MiniForm. (I had to change its name several times since I'm not very good with names and most of the decent names are not available).

MiniForm allows the following code:

 class RegistrationForm  
  include ActiveModel::Model

  attr_reader :user, :account

  attr_accessor :first_name, :last_name, :email, :name, :plan, :terms_of_service

  # user validation
  validates :first_name, presence: true
  validates :last_name, presence: true
  validates :email, presence: true, email: true

  # account validation
  validates :account_name, presence: true

  # form custom validation
  validates :plan, inclusion: {in AccountPlan::VALUES}
  validates :terms_of_service, acceptance: true

  # ensure uniqueness
  validate :ensure_unique_user_email
  validate :ensure_unique_account_name

  def initialize
    @user    = User.new
    @account = Account.new owner: @user
  end

  def update(attributes)
    attributes.each do |name, value|
      public_send "#{name}=", value
    end

    if valid? & user.update(user_attributes) && account.update(account_attributes)
      account.users << user
      AccountWasCreated.perform_later(account)
    else
      false
    end
  end

  private

  def ensure_unique_user_email
    errors.add :email, 'already taken' if User.where(email: email).any?
  end

  def ensure_unique_account_name
    errors.add :name, 'already taken' if Account.where(name: name).any?
  end

  def user_attributes
    {first_name: first_name, last_name: last_name, email: email}
  end

  def account_attributes
    {plan: plan, name: name}
  end
end  

To become:

class RegistrationForm  
  include MiniForm::Model

  # `model` delegates attributes to models 
  # and copies the validations from them
  model :user, attributes: %i(first_name last_name email), save: true
  model :account, attributes: %i(name plan), save: true

  attributes :terms_of_service

  validates :plan, inclusion: {in AccountPlan::VALUES}
  validates :terms_of_service, acceptance: true

  def initialize
    @user    = User.new
    @account = Account.new owner: @user
  end

  # `update` calls `perform` if model is valid
  # and models are saved
  def perform
    account.users << user
    AccountWasCreated.perform_later(account)
  end
end