Giter Site home page Giter Site logo

approvaltests.ruby's Issues

Newline on end of file

I have a small cosmetic issue. Approval files don't end on newlines. this means that if someone ever opens them in a properly configured editor, the file changes. It also means, that on github it shows a 'you-forgot-the-newline-at-the-end-of-file'. What are other peoples thoughts on this?

I would prefer a newline on the end of the approvals file

Html approver mangles html with boolean attributes

the following (valid) html:

<!DOCTYPE html>
<html>
<title>Hoi</title>
<script async defer src=\"http://foo.com/bar.js\"></script>
<h1>yo</h1>

becomes this:

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml"><script>window.FactlinkProxiedUri = "http://www.example.org/foo?bar=baz";</script><script></script>defer
               src="http://localhost:8000/lib/dist/factlink_loader.js?o=proxy"
               onload="__internalFactlinkState('proxyLoaded')"
         &gt;</html>

What is the preferred flow for accepting fixtures

This is more of a discussion than a direct issue. I noticed there is another workflow than I use, and wondered how other people are using this gem. I'll kick of by stating my method, and problems with it.

I check differences in file everyday. I have a perfect workflow for it, and I like it. This is my git commit tool (in my case gitx, but works for any other tool as well). I selected this tool especially for making it easy to see what has changed, so for me this is a perfect tool for comparing changes.

The problem is that to compare using my git tool, I need the approved file to change. To do this I have to move the file. Unfortunately, from the error output there is no straight-forward way to construct the move command (and due to the way this kind of stuff fails in my application, I regularly need to update multiple files). I'd like to be able to copy-paste something from the error output to my console, but this isn't totally possible due to trivial formatting differences between test output and mv command syntax.

To solve this I wrote a (bash) script which accepts all fixtures, and then check in my git client whether I want these changes, or whether this really indicates a bug.

The reason I started this discussion however is that I found a command inside the bin directory which provides similar functionality (though for myself I like the gui better than a commandline tool). So I was wondering, are other people using this commandline tool? Is it working good? If so, I think we should at least mention it in the readme ;) More generally, I think we should offer a bit better workflow in the readme than 'you move the file when you like it'.

BinaryWriter

What's the status of BinaryWriter? Is it still used? If I comment the whole file out, not a single test fails, and 'BinaryWriter' is mentioned nowhere else in the source.

Running approvals with parallel tests

We're using https://github.com/grosser/parallel_tests to speed up our integration tests. But this sometimes blows up, since the .approvals file is being reset by deleting it. This can happen at the same time when running parallel tests.

/home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/approvals-0.0.9/lib/approvals/utilities/dotfile.rb:7:in `delete': No such file or directory - /home/jenkins/workspace/PhabricatorCore/.approvals (Errno::ENOENT)
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/approvals-0.0.9/lib/approvals/utilities/dotfile.rb:7:in `reset'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/approvals-0.0.9/lib/approvals.rb:24:in `reset'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/approvals-0.0.9/lib/approvals.rb:29:in `<top (required)>'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/bundler-1.3.5/lib/bundler/runtime.rb:72:in `require'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/bundler-1.3.5/lib/bundler/runtime.rb:72:in `block (2 levels) in require'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/bundler-1.3.5/lib/bundler/runtime.rb:70:in `each'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/bundler-1.3.5/lib/bundler/runtime.rb:70:in `block in require'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/bundler-1.3.5/lib/bundler/runtime.rb:59:in `each'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/bundler-1.3.5/lib/bundler/runtime.rb:59:in `require'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/bundler-1.3.5/lib/bundler.rb:132:in `require'
    from /home/jenkins/workspace/PhabricatorCore/config/application.rb:11:in `<top (required)>'
    from /home/jenkins/workspace/PhabricatorCore/config/environment.rb:2:in `require'
    from /home/jenkins/workspace/PhabricatorCore/config/environment.rb:2:in `<top (required)>'
    from /home/jenkins/workspace/PhabricatorCore/spec/integration/spec_helper.rb:3:in `require'
    from /home/jenkins/workspace/PhabricatorCore/spec/integration/spec_helper.rb:3:in `<top (required)>'
    from /home/jenkins/workspace/PhabricatorCore/spec/integration/backend/acls/activities/bug_triaged_spec.rb:1:in `require'
    from /home/jenkins/workspace/PhabricatorCore/spec/integration/backend/acls/activities/bug_triaged_spec.rb:1:in `<top (required)>'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/rspec-core-2.13.1/lib/rspec/core/configuration.rb:819:in `load'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/rspec-core-2.13.1/lib/rspec/core/configuration.rb:819:in `block in load_spec_files'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/rspec-core-2.13.1/lib/rspec/core/configuration.rb:819:in `each'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/rspec-core-2.13.1/lib/rspec/core/configuration.rb:819:in `load_spec_files'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/rspec-core-2.13.1/lib/rspec/core/command_line.rb:22:in `run'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/rspec-core-2.13.1/lib/rspec/core/runner.rb:80:in `run'
    from /home/jenkins/.rbenv/versions/2.0.0-p353/lib/ruby/gems/2.0.0/gems/rspec-core-2.13.1/lib/rspec/core/runner.rb:17:in `block in autorun'

A hash ( # ) should not be stripped from approval name

I noticed I test a lot of controller actions, so my names tend to be something like

"blogposts#show.json should stay the same"

I think we can do better handling of the # than stripping it. (this would mean a breaking change though, but I think this is still okay, as long as we add it to the changelog)

rspec extension specs don't clean up after themselves

The tests in spec/extensions/rspec_approvals_spec.rb lines 64 - 78 leave the *received.txt files even when the tests pass. They also add entries in the .approvals file.

This gets in the way when there are other failures and you're using the command line tool to step through each of the diffs one at a time.

$ cat .approvals 
spec/fixtures/approvals/verifies_a_failure.approved.txt spec/fixtures/approvals/verifies_a_failure.received.txt
spec/fixtures/approvals/verifies_a_failure_diff.approved.txt spec/fixtures/approvals/verifies_a_failure_diff.received.txt
spec/fixtures/approvals/verifies_directory/a_failure.approved.txt spec/fixtures/approvals/verifies_directory/a_failure.received.txt
spec/fixtures/approvals/verifies_directory/a_failure_diff.approved.txt spec/fixtures/approvals/verifies_directory/a_failure_diff.received.txt

Some example metadata is wrong/missing (RSpec integration)

A bunch of the metadata on the example points to the DSL module rather than the place where the verify is being called, since specify() is being called in the DSL module. I think it should be possible to bind the call to specify to the callers context.

Some of the issues this causes are:

  • The approvals don't register :filtered => true, e.g. if you try to run just certain examples or groups.
  • The formatter for an example group points to the DSL file rather than the spec file

Differences in jruby and mri json output

I often have the problem that there are minor differences between the output of an approval test in jruby and mri. We run our tests in both rubies so the approval test always fails for such cases in one of the rubies.

An example of this is that the following generates different approvals in jruby and mri:

verify(format: :json) { { weight: {} } }

In jruby:

weight: {

}

in mri:

weight: {
}

Is there any way to avoid this?

The problem seems to be jsons pretty_generate: https://github.com/kytrinyx/approvals/blob/master/lib/approvals/writers/json_writer.rb#L11

jruby:

JSON.pretty_generate({ weight: {} })
=> "{\n  \"weight\": {\n\n  }\n}"`

mri:

JSON.pretty_generate({ weight: {} })
=> "{\n  \"weight\": {\n  }\n}"

RSpec 3 compatibility

Hey so the gem does not work under RSpec 3. It has to do with example being yielded into blocks instead of immediately available on the file itself.

I am looking into it but if you have any suggestions I am all ears.

The line that goes wrong is the following:

# /Users/mhenrixon/code/github/approvals/lib/approvals/extensions/rspec/dsl.rb:12:in 'verify'

Redifine match behaviour for filters?

I wonder why the choice was made to make a filter match regularly, instead of requiring it to match the whole string. Compare:

filters = {
  time: /^time$/,
  id: /(^|_)id$/,
  timestamp: /_at$/
}

versus

filters = {
  time: /time/,
  id: /(.*_)?id/,
  timestamp: /.*_at/
}

Imo the latter describe much clearer the content of the string matched, whereas the former sound more cryptical, and is easier to make mistakes with, like http://rubular.com/r/EnyxkU60Jo

Therefore I would be in favour of doing all-string matches always.

Add JRuby to travis build

It seems approvals works on JRuby, but outputs slightly different XML/HTML approvals. This shouldn't be to hard to fix.

Differences in JRuby and MRI HTML output

Similar to issue #84 there are also differences in the HTML output between JRuby and MRI.

Example:

describe "a html response" do
  let(:test_html) do
<<-TEST
<!doctype html>
<head>
<title>Test Title</title>
<meta charset="utf-8"/>
</head>
<body>
<h1>Test Page</h1>
</body>
</html>
TEST
end

  it "a test" do
    verify do
      test_html
    end
  end
end

MRI Output
MRI: 2.5.3
Approvals: 0.0.24
Nokogiri: 1.10.1

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
    <title>Test Title</title>
    <meta charset="utf-8" />
  </head>
  <body>
<h1>Test Page</h1>
</body>
</html>

JRuby Output
JRuby: 9.2.5.0
Approvals: 0.0.24
Nokogiri: 1.10.1-java

<!DOCTYPE html >
<html>
  <head>
    <title>Test Title</title>
    <meta charset="utf-8" />

  </head>
  <body>
    <h1>Test Page</h1>
  </body>
</html>

Workaround
A workaround is to use the format: :txt options, which produces the same (unchanged) output for both Ruby versions.

<!doctype html>
<head>
<title>Test Title</title>
<meta charset="utf-8"/>
</head>
<body>
<h1>Test Page</h1>
</body>
</html>

As far as I can tell, the issue is related to this line of code (https://github.com/kytrinyx/approvals/blob/master/lib/approvals/writers/html_writer.rb#L10), where Nokogiri produces a different output for each of the two Ruby versions.

Update readme

I was browsing around the code and am amazed at the amount of functionality which I didn't see from the readme. I think it would be really worthwhile to update the readme with the functionality at least, even if it isn't very well written/with lots of examples for now. I really think this is a great gem, and it apparantly is even better than I thought :)

Initialization issue w/ Approvals::CLI in v0.0.13

Cool idea for a library & I remember @rking being interested by this concept a year or so ago.

I ran into trouble w/ v0.0.13:

  • When trying to run the cmdline tool, it complains about:
$ bundle exec approvals                                                                                        
/Users/zph/projectX/vendor/ruby/1.9.1/gems/approvals-0.0.13/bin/approvals:7:in `<top (required)>': uninitialized constant Approvals (NameError)
        from /Users/zph/rentpath/source/rnr_ui/vendor/ruby/1.9.1/bin/approvals:23:in `load'
        from /Users/zph/rentpath/source/rnr_ui/vendor/ruby/1.9.1/bin/approvals:23:in `<main>'

So, I reverted to 0.0.12 in my Gemfile and it's working as expected.

After I fix the issue that I'm hoping to refactor, I'll see about digging into this further 😄.

Thanks for putting this lib out there!

Blows up on array of hashes

I have an array of hashes that looks like this:

people = [
  {"name" => "Alice", "age" => 28},
  {"name" => "Bob", "age" => 22}
]

When I say Approvals.verify(people, name: 'people', format: :json) it blows up.

AssignmentTest#test_files:
TypeError: can't convert Array into String
    /opt/rubies/ruby-1.9.3-p392/lib/ruby/1.9.1/json/common.rb:148:in `initialize'
    /opt/rubies/ruby-1.9.3-p392/lib/ruby/1.9.1/json/common.rb:148:in `new'
    /opt/rubies/ruby-1.9.3-p392/lib/ruby/1.9.1/json/common.rb:148:in `parse'
    /Users/kytrinyx/.gem/ruby/1.9.3/gems/approvals-0.0.9/lib/approvals/writers/json_writer.rb:24:in `parse_data'
    /Users/kytrinyx/.gem/ruby/1.9.3/gems/approvals-0.0.9/lib/approvals/writers/json_writer.rb:10:in `format'
    /Users/kytrinyx/.gem/ruby/1.9.3/gems/approvals-0.0.9/lib/approvals/writers/text_writer.rb:13:in `block in write'
    /Users/kytrinyx/.gem/ruby/1.9.3/gems/approvals-0.0.9/lib/approvals/writers/text_writer.rb:12:in `open'
    /Users/kytrinyx/.gem/ruby/1.9.3/gems/approvals-0.0.9/lib/approvals/writers/text_writer.rb:12:in `write'
    /Users/kytrinyx/.gem/ruby/1.9.3/gems/approvals-0.0.9/lib/approvals/approval.rb:42:in `verify'
    /Users/kytrinyx/.gem/ruby/1.9.3/gems/approvals-0.0.9/lib/approvals/dsl.rb:4:in `verify'

Undocumented dependency/require

Perhaps this is a mistake on my part, but when I use approvals in a minitest environment and attempt to match a approval and the received file, there is a requirement on 'ERB' that is undocumented in approvals source.

Simply adding require 'erb' to my test/test_helper.rb fixed the problem. I haven't pushed the code I was working on at that moment yet, but it is on a public repo so I could show when I push later today.

consolidate configuration behavior

Comment from @markijbema in #69

Also, do I understand correctly (been a while since I used approvals) that there are two ways to configure Approvals? That is suboptimal at least imho.

I don't actually know the answer to this. We should take a look, and if it's true we should normalize.

Make 'json' an explicit dependency

Currently the json dependency is installed here.

Ruby 1.x already reached the end of the support a long time ago. I think the json dependency should be explicitly listed by now.

Using approvals to verify_sql

I've had good luck with the following pattern in my apps and I'm curious if there would be any interest in wrapping this up into a new feature for the gem as we have the verify helper. No worries in any case, but I thought I'd share the pattern here for other people to consider. I find this useful for watching the SQL that complex scopes generate. It also comes in handy when working with GraphQL to ensure that common queries generate reasonable SQL. The end result is something akin to what the bullet gem does, but a bit more specialized, and with an easily readable record that prevents accidental N+1s etc.

# spec/spec_helper.rb

require "approvals/rspec"
require "./spec/support/approvals_helper"

RSpec.configure do |config|
  config.include ApprovalsHelper
  config.approvals_default_format = :txt
end
# spec/support/approvals_helper.rb

module ApprovalsHelper
  def verify_sql(&block)
    sql = []

    subscriber = ->(_name, _start, _finish, _id, payload) do
      sql << payload[:sql].split("/*").first.gsub(/\d+/, "?")
    end

    ActiveSupport::Notifications.subscribed(subscriber, "sql.active_record", &block)

    verify do
      sql.join("\n") + "\n"
    end
  end
end
# spec/models/example_spec.rb

it "is an example spec" do
   verify_sql do
     expect(
       Thing.complex_query
     ).to eq(
       expected_things
     )
   end
 end

Mutual incompatibility with Rails 6.1.x over versions of `thor` 

Bundler could not find compatible versions for gem "thor":
  In Gemfile:
    approvals (~> 0.0.24) was resolved to 0.0.24, which depends on
      thor (~> 0.18)

    factory_bot_rails was resolved to 6.2.0, which depends on
      railties (>= 5.0.0) was resolved to 6.1.3.2, which depends on
        thor (~> 1.0)

Any chance approvals can be updated to permit newer versions of thor?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.