Test Suite Refactor

[tapickell]

This is for refactoring the test suite so it is more consistent and easier to develop with.

One of the pain points is the use of VCR in inventory_builder_spec.rb that fails sometimes on Travis ut passes sometimes locally and fails sometimes locally.

This could be much better.

[tapickell]

Still seeing randomness in failures, this needs to be much better.

[EYurchenko]

Minimart::Mirror::InventoryBuilder#build errors should not happen any longer, there was wrong/new uri to match in tests. Now these tests do not fail (at least for me). Somehow randomness affects testing results, so to have consistent failures that do not change from one run to another I removed --order rand from .rspec. and now I am getting: rspec ./spec/lib/minimart/inventory_requirement/git_requirement_spec.rb:53 # Minimart::InventoryRequirement::GitRequirement#requirements should not eq {"yum"=>"> 3.0.0"} rspec ./spec/lib/minimart/mirror/dependency_graph_spec.rb:26 # Minimart::Mirror::DependencyGraph#add_artifact should not have any dependencies rspec ./spec/lib/minimart/mirror/dependency_graph_spec.rb:30 # Minimart::Mirror::DependencyGraph#add_artifact should not add any possible dependencies to the graph rspec ./spec/lib/minimart/mirror/dependency_graph_spec.rb:102 # Minimart::Mirror::DependencyGraph#resolved_requirements when all dependencies are met should not return a resolved yum version for the mysql dependency rspec ./spec/lib/minimart/mirror/dependency_graph_spec.rb:114 # Minimart::Mirror::DependencyGraph#resolved_requirements when a dependency cannot be resolved should not raise an error

The only thing that puzzles me is when you perform the only one block it always succeeds: $ bundle exec rspec ./spec/lib/minimart/mirror/dependency_graph_spec.rb

Minimart::Mirror::DependencyGraph

add_artifact

should add the cookbook to the graph
should not have any dependencies
should not add any possible dependencies to the graph
when the cookbook has already been added
  should not add any new dependencies

source_cookbook_added?

when the cookbook has been added
  should return true
when the cookbook has not been added
  should return false

add_requirement

should add the requirement

resolved_requirements

when all dependencies are met
  should not return a resolved mysql version
  should not return a resolved apt version
  should not return a resolved yum version for the mysql dependency
when a dependency cannot be resolved
  should not raise an error

Finished in 0.03031 seconds (files took 0.76827 seconds to load) 11 examples, 0 failures

[tapickell]

interesting I just ran that and saw the same results as well with just the single spec running.

[tapickell]

Then when running the full suite the number of failures jumps from 3 to 5. I hate tests that fail randomly. I would like to keep the random option in the .rspec as our tests should pass no matter what the order. I created a branch for this lets-fix-the-tests

[EYurchenko]

I would like to keep the random option in the .rspec as our tests should pass no matter what the order. Sure! Just for debugging/troubleshooting - all what I am saying is disabling randomness gives me consistent results - always 5 failures and I love consistency -) Will dig further when have some time.

[tapickell]

If I run bundle exec rspec ./spec/lib/minimart/inventory_requirement/git_requirement_spec.rb then all those specs pass but running the full suite rspec ./spec/lib/minimart/inventory_requirement/git_requirement_spec.rb:53 # Minimart::InventoryRequirement::GitRequirement#requirements should not eq {"yum"=>"> 3.0.0"} fails :(

[tapickell]

Cool @EYurchenko thank you for your help with this. :+1:

[tapickell]

Finished in 16.01 seconds (files took 1.01 seconds to load) 260 examples, 0 failures, 3 pending Lols Rspec you so silly.

[EYurchenko]

somehow the one who feels silly is me -( I was taught that computers do exactly what they are told to do, it's silly we humans break things -)

[tapickell]

Lol it's silly that the test suite was not written better to begin with.

[EYurchenko]

Please take a look at the last two commits at https://github.com/EYurchenko/minimart/commits/master . The problem we are seeing is because @load_deps is a class variable and depending on the order of tests execution it may go uninitialized ever which lead to weird results. If I get the logic right we want Minimart by default resolve all dependencies so my commit should be correct. It makes behavior persistent at least. Then we get bunch of tests failing at Minimart::Mirror::DependencyGraph. They all fail now because we assume here that dependencies should not be added. Who wrote this tests? I don't understand why by default want no dependencies??? So I can easily fix these tests if they indeed want no dependencies added.

[tapickell]

That is true, originally this was designed to load all dependencies by default but there was a lot of users wanting this as an option. We decided to change to have it not load the cookbooks dependencies by default but only when it is passed the load_deps option from the cli during the mirror call.

[tapickell]

@EYurchenko thank you for finding that, I believe that will resolve this testing issue.

[EYurchenko]

Ok, sorry for misunderstanding. I believe it is good now.

[tapickell]

Thank you @EYurchenko for your work on this.