RSpec-Results of reviewing the test code for'users validation'

Introduction

Hello everyone! Qiita's post, ruby, rails, rspec I'm new to J! Just the other day, I was worried about "What about my spec from a professional perspective?" I posted RSpec-Testing'users validation' as a question. And videos about ruby and rspec are distributed on youtube, I had my teacher @jnchito do a code review! The test code was fairly long, but I asked them to point out each one thoroughly. He politely explained "I would write this!" Thanks to you,

I got lost and wondered "what is the test code that follows the rules?" I can't commit easily and I'm wandering around hell.

I feel like I'm in heaven now! Thank you very much for your review. .. Crying

About @jnchito

It's famous, so you may know it, but ...

-Introduction to Ruby for those who want to become a professional I have published a book about ruby, -Everyday Rails-Introduction to Rails Testing with RSpec Translate books about rspec into Japanese, -Try to extract only the Twitter account in the text with a regular expression There were also videos that were distributed on youtube and held study sessions! -(1/2) I reviewed the RSpec test code posted on Qiita Click here for the video reviewed this time!

A really cool dandy teacher! !!

So this article is also an output for me to become a specker, To be a reference for comrades who still have the same skill as me From what was pointed out and explained, I looked back at my own code and tried to fix the bad points! Please refer to it if you like!

My garbage code before correction

If you put the before and after test code with the previous code, the Qiita database may be punctured. ← (・ ・.) So, if you want to compare My garbage code before modification, please duplicate the tab!

Nested image of Spec

"Isn't it necessary for the nest to be too deep?", So I corrected it as shallowly as possible!

RSpec.describe User, type: :model do
  describe 'attribute: name'
    context 'when present'
      it 'is valid'
        # ...abridgement
      end
    end
  end
end

The above content is, ʻUser> name> when present> it is valid` It sounds strange in English translation, but every nest is ... Re-nested to make it easier to understand with the nuance of "valid if the user name exists". This is just an image!

Revised

User model

app/models/user.rb

class User < ApplicationRecord
  before_save :downcase_email

  validates :name,
            presence: true,
            length: { maximum: 50 }
  VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-]+(\.[a-z\d\-]+)*\.[a-z]+\z/i
  validates :email,
            presence: true,
            length: { maximum: 255 },
            format: { with: VALID_EMAIL_REGEX },
            uniqueness: { case_sensitive: false }

  private

    def downcase_email
      self.email = self.email.downcase
    end
end

Supplement

• VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-]+(\.[a-z\d\-]+)*\.[a-z]+\z/i
  From railstutorial "Do not match two consecutive dots" Corrected to this regular expression. --self.email.downcase! And destructive methods are rarely used. It is safer not to use destructive methods as it is unclear when and where they will be referenced in the future. ,,,Certainly. (So it has been fixed below.)

def downcase_email
  self.email = self.email.downcase
end

FactoryBot

spec/factories/users.rb

FactoryBot.define do
  factory :user do
    name { 'Tony Stark' }
    email { '[email protected]' }
  end
end

Supplement

--Before modification: name {'hoge'}, ʻemail {"[email protected]"} The domain part of the email address that I thought was impossible. .. Oh, that? [bar.com](https://www.bar.com/) Click! I swear to use ,,,@ example.com`.

Spec

spec/models/user_spec.rb

require 'rails_helper'

RSpec.describe User, type: :model do
  let(:user) { FactoryBot.build(:user) }

  describe 'attribute: name' do
    context 'when present' do
      #If name exists, it must be valid
      it 'is valid' do
        user.name = 'Tony Stark'
        expect(user).to be_valid
      end
    end

    context 'when blank' do
      #If name is blank, it is invalid
      it 'is invalid' do
        user.name = ' '
        expect(user).to be_invalid
        expect(user.errors[:name]).to include("can't be blank")
      end
    end

    context 'when empty' do
      #Invalid if name is empty
      it 'is invalid' do
        user.name = ''
        expect(user).to be_invalid
        expect(user.errors[:name]).to include("can't be blank")
      end
    end

    context 'when nil' do
      #If name does not exist, it is invalid
      it 'is invalid' do
        user.name = nil
        expect(user).to be_invalid
        expect(user.errors[:name]).to include("can't be blank")
      end
    end

    context 'when length is 50 characters or less' do
      #Must be valid if name is 50 characters or less
      it 'is valid' do
        user.name = 'a' * 50
        expect(user).to be_valid
      end
    end

    context 'when length is more than 50 characters' do
      #Invalid if name exceeds 50 characters
      it 'is invalid' do
        user.name = 'a' * 51
        expect(user).to be_invalid
        expect(user.errors[:name]).to include('is too long (maximum is 50 characters)')
      end
    end
  end

  describe 'attribute: email' do
    context 'when present' do
      #If the email exists, it must be valid
      it 'is invalid' do
        user.email = '[email protected]'
        expect(user).to be_valid
      end
    end

    context 'when blank' do
      #If email is blank, it is invalid
      it 'is invalid' do
        user.email = ' '
        expect(user).to be_invalid
        expect(user.errors[:email]).to include("can't be blank")
      end
    end

    context 'when empty' do
      #Invalid if email is empty
      it 'is invalid' do
        user.email = ''
        expect(user).to be_invalid
        expect(user.errors[:email]).to include("can't be blank")
      end
    end

    context 'when nil' do
      #Invalid if email does not exist
      it 'is invalid' do
        user.email = nil
        expect(user).to be_invalid
        expect(user.errors[:email]).to include("can't be blank")
      end
    end

    context 'when length is 50 characters or less' do
      #If the email is 255 characters or less, it must be valid
      it 'is valid' do
        user.email = 'a' * 243 + '@example.com'
        expect(user).to be_valid
      end
    end

    context 'when length is more than 50 characters' do
      #If the email exceeds 255 characters, it is invalid
      it 'is invalid' do
        user.email = 'a' * 244 + '@example.com'
        expect(user).to be_invalid
        expect(user.errors[:email]).to include('is too long (maximum is 255 characters)')
      end
    end

    context 'when correct format' do
      #If the email format is correct, it is valid
      it 'is valid' do
        user.email = '[email protected]'
        expect(user).to be_valid

        user.email = '[email protected]'
        expect(user).to be_valid

        user.email = '[email protected]'
        expect(user).to be_valid

        user.email = '[email protected]'
        expect(user).to be_valid

        user.email = '[email protected]'
        expect(user).to be_valid
      end
    end

    context 'when is incorrect format' do
      #Invalid if the email format is incorrect
      it 'is invalid' do
        user.email = 'user@example,com'
        expect(user).to be_invalid

        user.email = 'user_at_foo.org'
        expect(user).to be_invalid

        user.email = 'user.name@example.'
        expect(user).to be_invalid

        user.email = 'foo@bar_baz.com'
        expect(user).to be_invalid

        user.email = 'foo@bar+baz.com'
        expect(user).to be_invalid
      end
    end

    context 'when already taken' do
      #If the same email is already registered, it must be invalid
      it 'is invalid' do
        FactoryBot.create(:user, email: '[email protected]')
        user.email = '[email protected]'
        expect(user).to be_invalid
        expect(user.errors[:email]).to include('has already been taken')
      end
    end

    context 'when case insensitive and not unipue' do
      #Email is not case sensitive and is invalid if it is not unique
      it 'is invalid' do
        FactoryBot.create(:user, email: '[email protected]')
        user.email = '[email protected]'
        expect(user).to be_invalid
        expect(user.errors[:email]).to include('has already been taken')
      end
    end

    #The email is saved in lowercase
    it 'is saved in lowercase' do
      user.email = '[email protected]'
      user.save!
      expect(user.reload.email).to eq '[email protected]'
    end
  end
end

Supplement

--Teacher: Delete unnecessary debug codes. Me: Ah, yes! But when testing, pry is useful! --Teacher: It's not good to have to go see the users of FactoryBot. Me: Even if you define a user in FactoryBot, explicitly put a value in name before writing the exspection! --I: Corrected the text of context and ʻit! --context:'when ~'group when ~ --it: Validate valid pattern Validate invalid pattern The image is the nuance of the context of the part of the nested image. ――Teacher: I wonder if you don't need to verify save. .. It's rare that validation goes through and saves, and if there is a rails bug or something. Me: I see! (Deleted or less.) Excerpt from unmodified code: ʻexpect (user.save) .to be_falsey --Teacher: Let's summarize the test here. "Invalid if name does not exist" -- name =''' (space only) --name ='' (empty)

• name = nil（nil）

    context 'when name is blank' do
      #If name does not exist, it is invalid
      it 'is invalid' do
        user.name = ' '
        expect(user).to be_invalid
        expect(user.errors[:name]).to include("can't be blank")

        user.name = ''
        expect(user).to be_invalid
        expect(user.errors[:name]).to include("can't be blank")

        user.name = nil
        expect(user).to be_invalid
        expect(user.errors[:name]).to include("can't be blank")
      end
    end

Me: I wanted to write this without putting it together, so I divided it into blank, ʻempty, and nil`!

--I: I thought that messages would not be included in the error message unless I asked ʻuser.valid?, So, just write ʻexpect without asking ʻuser.valid?! ――Teacher: Regular expression test, this is good! Me: Azasu! --Teacher: Throw away dry and store it in a variable and don't do this or that! If you build a logic, a mistake will occur in that logic and an unexpected accident will occur. .. .. --Teacher: ʻuniqueness: {case_sensitive: false} Should I test it? Me: (2/2) I reviewed the RSpec test code posted on Qiita As you can see, --Save valid users with lowercase email addresses. --Re-enter the user's uppercase email registered above into the user's email. --user is invalid. --Also, the error message contains has already been taken. Is the code below.

context 'when case insensitive and not unipue' do
  #Email is not case sensitive and is invalid if it is not unique
  it 'is invalid' do
    FactoryBot.create(:user, email: '[email protected]')
    user.email = '[email protected]'
    expect(user).to be_invalid
    expect(user.errors[:email]).to include('has already been taken')
  end
end

Summary

I got a lot more points, but I haven't reached the professional level yet, so I will review the video that you reviewed and explained and describe the part that I understood more clearly! It seems that there are many comrades who start writing tests and do not know how much to "test" like me. I think they have different opinions, but it's a beginner's job!

Believe in'rails validation and test your own and others' logic! '

That's right! I thought it was a waste to bother to test rails verification ... Also, as the teacher said, there are some things that you can't say when you say it. I think the ability to narrow down the test points is more important. I think that there is a correct test code that can be said to be almost 100% for the test for this validation, so It's going to be a future practice to test the logic you and your team members have created!

Then everyone!

Discard dry for testing unless you need it!

Everyone can read the code easily and be happy More than anything,, It doesn't make sense if TEST is wrong, w

see you!

Test output

User
  attribute: name
    when present
      is valid
    when blank
      is invalid
    when empty
      is invalid
    when nil
      is invalid
    when length is 50 characters or less
      is valid
    when length is more than 50 characters
      is invalid
  attribute: email
    is saved in lowercase
    when present
      is invalid
    when blank
      is invalid
    when empty
      is invalid
    when nil
      is invalid
    when length is 50 characters or less
      is valid
    when length is more than 50 characters
      is invalid
    when correct format
      is valid
    when is incorrect format
      is invalid
    when already taken
      is invalid
    when case insensitive and not unipue
      is invalid