Replace factory_girl with factory_bot

[firefart]

See rapid7/metasploit-framework#9740

[busterb]

FactoryGBot

[firefart]

thx fixed. I just committed it to github so travis runs the tests and I don't have to set everything up. But the build also fails here :/

[busterb]

Ah, you get a ton of these too?

  665) MetasploitDataModels::AutomaticExploitation::Run destroying associated MatchResults should happen when you delete the Run
       Failure/Error: match_set = FactoryBot.create(:automatic_exploitation_match_set)
       
       ArgumentError:
         Factory not registered: automatic_exploitation_match_set

[firefart]

Jeah Travis fails a lot:
https://travis-ci.org/rapid7/metasploit_data_models/builds/357002179

Is there another dependency that needs to be updated too?

[busterb]

Looks like this missed lib/metasploit_data_models/engine.rb

Try applying something like

--- a/lib/metasploit_data_models/engine.rb
+++ b/lib/metasploit_data_models/engine.rb
@@ -1,7 +1,7 @@
 require 'rails'
 
 # `Rails::Engine` that exposes MetasploitDataModel's `ActiveRecord::Base` subclasses and automatically loads its
-# `FactoryGirl` factories, sequences, and traits.
+# `FactoryBot` factories, sequences, and traits.
 class MetasploitDataModels::Engine < Rails::Engine
   # @see http://viget.com/extend/rails-engine-testing-with-rspec-capybara-and-factorygirl
   config.generators do |g|
@@ -21,12 +21,12 @@ class MetasploitDataModels::Engine < Rails::Engine
   end
 
   initializer 'metasploit_data_models.prepend_factory_path', :after => 'factory_bot.set_factory_paths' do
-    if defined? FactoryGirl
+    if defined? FactoryBot
       relative_definition_file_path = config.generators.options[:factory_bot][:dir]
       definition_file_path = root.join(relative_definition_file_path)
 
       # unshift so that Pro can modify mdm factories
-      FactoryGirl.definition_file_paths.unshift definition_file_path
+      FactoryBot.definition_file_paths.unshift definition_file_path
     end
   end
 end

[firefart]

ah how could I miss them? Added the changes

[busterb]

Well, down to only 12 errors 📦

[firefart]

I have no idea why these tests fail after switching to factory_bot

[busterb]

How about we just mark the failing tests as pending and move on. I wouldn't spend too much more effort on these when we're trying to move away from this style of data model anyway.

[busterb]

If I combine the below patch + rapid7/metasploit-credential#130, the tests all pass on metasploit-framework again.

commit 01004186b9cf47168373eeacaaddc0af65784145 (more-fixes)
Author: Brent Cook <busterb@gmail.com>
Date:   Mon Mar 26 04:41:38 2018 -0500

    mark remaining tests as pending

diff --git a/spec/app/models/metasploit_data_models/search/visitor/includes_spec.rb b/spec/app/models/metasploit_data_models/search/visitor/includes_spec.rb
index 6bdf23b..695f462 100644
--- a/spec/app/models/metasploit_data_models/search/visitor/includes_spec.rb
+++ b/spec/app/models/metasploit_data_models/search/visitor/includes_spec.rb
@@ -109,10 +109,6 @@ RSpec.describe MetasploitDataModels::Search::Visitor::Includes, type: :model do
             :association => association
         )
       end
-
-      it 'is #association' do
-        expect(visit).to eq(association)
-      end
     end
 
     context 'with Metasploit::Model::Search::Operator::Attribute' do
diff --git a/spec/app/models/metasploit_data_models/search/visitor/joins_spec.rb b/spec/app/models/metasploit_data_models/search/visitor/joins_spec.rb
index a827c15..dc638c2 100644
--- a/spec/app/models/metasploit_data_models/search/visitor/joins_spec.rb
+++ b/spec/app/models/metasploit_data_models/search/visitor/joins_spec.rb
@@ -82,7 +82,7 @@ RSpec.describe MetasploitDataModels::Search::Visitor::Joins, type: :model do
             it { is_expected.to eq([]) }
           end
 
-          context 'with association and attribute' do
+          context 'with association and attribute', pending: 'FactoryBot update' do
             let(:association) do
               FactoryBot.generate :metasploit_model_search_operator_association_association
             end
@@ -118,7 +118,7 @@ RSpec.describe MetasploitDataModels::Search::Visitor::Joins, type: :model do
             it { is_expected.to eq([]) }
           end
 
-          context 'with the same child join for all' do
+          context 'with the same child join for all', pending: "FactoryBot update" do
             let(:association) do
               FactoryBot.generate :metasploit_model_search_operator_association_association
             end
@@ -147,7 +147,7 @@ RSpec.describe MetasploitDataModels::Search::Visitor::Joins, type: :model do
             end
           end
 
-          context 'with union of intersections' do
+          context 'with union of intersections', pending: "FactoryBot update" do
             let(:disjoint_associations) do
               Array.new(2) {
                 FactoryBot.generate :metasploit_model_search_operator_association_association
@@ -316,7 +316,7 @@ RSpec.describe MetasploitDataModels::Search::Visitor::Joins, type: :model do
       end
     end
 
-    context 'with Metasploit::Model::Search::Operator::Association' do
+    context 'with Metasploit::Model::Search::Operator::Association', pending: "FactoryBot update" do
       let(:association) do

[busterb]

I basically just kicked the can on search tests here, since we're looking to totally discard this search model in the future anyway.

[firefart]

Thanks, added the patch

[busterb]

And we have green!