Skip to content

Advanced specs to test code #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

bpurinton
Copy link
Contributor

@bpurinton bpurinton commented Apr 6, 2023

So far, the previously added specs just test basic functionality.

A better way might be actually testing for the presence of the helper methods in the source code, rather than just testing the HTML output.

Things to potentially test for here:

  • presence of link_to
  • presence of form_with in new.html.erb and edit.html.erb
  • presence of find in controller
  • removal of query_ from form params and change from strings to symbols
  • presence of bundled sub-hashes in forms and mass assignment in controllers
  • presence of resources in routes.rb

An unconventional suggestion from @jelaniwoods that works well (and has already been added as a commit here):

require "rails_helper"

describe "movies/index" do
  it "uses an embedded Ruby link_to helper method", points: 2 do
    template_name = "movies/index.html.erb"
    template_path = Rails.root.join('app', 'views', template_name)
    template_contents = open(template_path).read
    expect(template_contents).to match /<%= link_to "Show details", movie %>/
  end
end

@jelaniwoods
Copy link
Contributor

If you are going to use Regex, you might want to consider ignoring some whitespace and/or variable names if those aren't important.

This test

    expect(template_contents).to match /<%= link_to "Show details", movie %>/

will fail for this view code because it has a trailing space at the end

    <%= link_to "Show details", movie  %>

It would also fail if someone named their variable m instead of movie or if parenthesis were used around arguments.

So it might be better to make the Regex more flexible, or be explicit about the requirements in the title and/or failure message.

The simplest refactor might just be to test for link_to in ERB tags and rely on a different test to ensure that the desired functionality still works.

This would change the Regex to something like

expect(template_contents).to match /<%=.*link_to.*"Show details".*%>/

@bpurinton bpurinton marked this pull request as draft April 6, 2023 20:58
@raghubetina
Copy link
Contributor

raghubetina commented Jun 12, 2023

Perhaps we could use receive? https://stackoverflow.com/a/21263118

@bpurinton
Copy link
Contributor Author

@raghubetina @jelaniwoods It might be time to re-open this question:

Do we want to implement Phase 2 specs that read the codebase and ensure that helper methods are being utilized?

Motivation:

DPI will be going through these projects in the near future and they are interested in grades / progress. The current specs for e.g. the helper methods projects are very basic and do not have any checks that they are actually using the helper methods.

Copy link
Contributor

@jelaniwoods jelaniwoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to implement Phase 2 specs that read the codebase and ensure that helper methods are being utilized?

I think that's a good idea.

You can use view specs and stubbing to explicitly check for methods being called in views. It's much more robust than regex testing.

# spec/views/movies/show.html.erb_spec.rb
require "rails_helper"

describe "movies/show.html.erb" do

  let!(:movie) { Movie.create(title: "E", description: "EE", created_at: 1.day.ago, updated_at: 0.day.ago) }

  it "uses time_ago_in_words for created_at" do
    assign(:the_movie, movie)
    allow(view).to receive(:time_ago_in_words)
    render
    expect(view).to have_received(:time_ago_in_words).with(movie.created_at)
  end

  it "uses link_to for edit link" do
    assign(:the_movie, movie)
    allow(view).to receive(:link_to)
    render
    expect(view).to have_received(:link_to).with("Edit Movie", "/movies/#{movie.id}/edit")
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants