pezra / rspec-mode

An RSpec minor mode for Emacs
257 stars 112 forks source link

Make as a hyperlink the screenshot path automatically taken by System Spec #193

Closed kzkn closed 4 years ago

kzkn commented 4 years ago

Make as a hyperlink the screenshot path automatically taken by RSpec's System Spec.

Example:

RSpec.describe "テスト", type: :system, js: true do
  it "内容が表示されること" do
    # this will be fail, system spec will take a screenshot automatically.
    expect(page).to have_content "ハロー"
  end
end

Outputs:

  1) テスト 内容が表示されること
     Failure/Error: expect(page).to have_content "ハロー"
       expected to find text "ハロー" in ""

     [Screenshot]: /home/kazuki/myproj/tmp/screenshots/failures_r_spec_example_groups_nested_2_内容が表示されること_237.png

This pull request makes the above path as a hyperlink.

dgutov commented 4 years ago

Hi!

What about the preceding rule? Does that format come from a different setup? Or a previous version of Capybara?

kzkn commented 4 years ago

@dgutov

  '((rspec-capybara-html "^ +HTML screenshot: \\([0-9A-Za-z@_./\:-]+\\.html\\)" 1 nil nil 0 1)
    (rspec-capybara-screenshot "^ +Image screenshot: \\([0-9A-Za-z@_./\:-]+\\.png\\)" 1 nil nil 0 1)

They are output formats by capybara-screenshot gem. It's still a good way in the old style of feature specs to take a screenshot automatically on fail a spec.

    (rspec-system-spec-screenshot "^ +\\[Screenshot\\]: \\(.+\\.png\\)" 1 nil nil 0 1)

This is an output format by System Test that built-in Rails. RSpec's System Spec is a wrapper of it.

dgutov commented 4 years ago

Makes sense, thank you. So both formats need to be supported.

Could you modify the previous rule, though, instead of adding a new one? They only differ in the first two words, I think. And brackets.

kzkn commented 4 years ago

@dgutov Thanks. Let me just check one thing. Do you mean merge rspec-capybara-html, rspec-capybara-screenshot and rspec-system-spec-screenshot? Or merge rspec-capybara-screenshot and rspec-system-spec-screenshot?

dgutov commented 4 years ago

Or merge rspec-capybara-screenshot and rspec-system-spec-screenshot?

Let's go with this one.

kzkn commented 4 years ago

@dgutov I did it :smiley:

dgutov commented 4 years ago

Could you keep the bol anchoring (^ +)? It's better for performance.

You might also want to use grouping and the alternation instruction (\\|).

kzkn commented 4 years ago

@dgutov thanks. I did it.

dgutov commented 4 years ago

Um, this part doesn't make sense: \\(\\|.

Let's revert that part to the previous version. But keep the bol anchoring.

kzkn commented 4 years ago

ok, did it.

dgutov commented 4 years ago

Thanks!