Update docs and description; Cleanup attributes52992-absorb-qa-product-qa

10 files changed, 31 insertions, 79 deletions

diff --git a/qa/qa/factory/README.md b/qa/qa/factory/README.md
index cfce096ab39..42077f60611 100644
--- a/qa/qa/factory/README.md
+++ b/qa/qa/factory/README.md
@@ -88,44 +88,6 @@ end
 The [`Project` factory](./resource/project.rb) is a good real example of Browser
 UI and API implementations.
 
-### Define attributes
-
-After the resource is fabricated, we would like to access the attributes on
-the resource. We define the attributes with `attribute` method. Suppose
-we want to access the name on the resource, we could change `attr_accessor`
-to `attribute`:
-
-```ruby
-module QA
-  module Factory
-    module Resource
-      class Shirt < Factory::Base
-        attribute :name
-
-        # ... same as before
-      end
-    end
-  end
-end
-```
-
-The difference between `attr_accessor` and `attribute` is that by using
-`attribute` it can also be accessed from the product:
-
-```ruby
-shirt =
-  QA::Factory::Resource::Shirt.fabricate! do |resource|
-    resource.name = "GitLab QA"
-  end
-
-shirt.name # => "GitLab QA"
-```
-
-In the above example, if we use `attr_accessor :name` then `shirt.name` won't
-be available. On the other hand, using `attribute :name` will allow you to use
-`shirt.name`, so most of the time you'll want to use `attribute` instead of
-`attr_accessor` unless we clearly don't need it for the product.
-
 #### Resource attributes
 
 A resource may need another resource to exist first. For instance, a project
@@ -145,7 +107,7 @@ module QA
   module Factory
     module Resource
       class Shirt < Factory::Base
-        attribute :name
+        attr_accessor :name
 
         attribute :project do
           Factory::Resource::Project.fabricate! do |resource|
@@ -206,7 +168,7 @@ module QA
   module Factory
     module Resource
       class Shirt < Factory::Base
-        attribute :name
+        attr_accessor :name
 
         attribute :project do
           Factory::Resource::Project.fabricate! do |resource|
@@ -287,7 +249,7 @@ module QA
             shirt_new.create_shirt!
           end
 
-          brand # Eagerly construct the data
+          populate(:brand) # Eagerly construct the data
         end
       end
     end
@@ -295,9 +257,12 @@ module QA
 end
 ```
 
-This will make sure we construct the data right after we created the shirt.
-The drawback for this will become we're forced to construct the data even
-if we don't really need to use it.
+The `populate` method will iterate through its arguments and call each
+attribute respectively. Here `populate(:brand)` has the same effect as
+just `brand`. Using the populate method makes the intention clearer.
+
+With this, it will make sure we construct the data right after we create the
+shirt. The drawback is that this will always construct the data when the resource is fabricated even if we don't need to use the data.
 
 Alternatively, we could just make sure we're on the right page before
 constructing the brand data:
@@ -307,7 +272,7 @@ module QA
   module Factory
     module Resource
       class Shirt < Factory::Base
-        attribute :name
+        attr_accessor :name
 
         attribute :project do
           Factory::Resource::Project.fabricate! do |resource|
@@ -385,7 +350,7 @@ end
 
 **Notes on attributes precedence:**
 
-- attributes from the factory have the highest precedence
+- factory instance variables have the highest precedence
 - attributes from the API response take precedence over attributes from the
   block (usually from Browser UI)
 - attributes without a value will raise a `QA::Factory::Base::NoValueError` error
diff --git a/qa/qa/factory/base.rb b/qa/qa/factory/base.rb
index 878bbee85c8..e28a00c545b 100644
--- a/qa/qa/factory/base.rb
+++ b/qa/qa/factory/base.rb
@@ -31,7 +31,7 @@ module QA
       def populate_attribute(name, block)
         value = attribute_value(name, block)
 
-        raise NoValueError, "No value was computed for product #{name} of factory #{self.class.name}." unless value
+        raise NoValueError, "No value was computed for #{name} of #{self.class.name}." unless value
 
         value
       end
diff --git a/qa/qa/factory/repository/project_push.rb b/qa/qa/factory/repository/project_push.rb
index a9dfbc0a783..272b7fc5818 100644
--- a/qa/qa/factory/repository/project_push.rb
+++ b/qa/qa/factory/repository/project_push.rb
@@ -9,8 +9,6 @@ module QA
           end
         end
 
-        attribute :output
-
         def initialize
           @file_name = 'file.txt'
           @file_content = '# This is test project'
diff --git a/qa/qa/factory/resource/merge_request.rb b/qa/qa/factory/resource/merge_request.rb
index e20ab0e67e2..4b7d2287f98 100644
--- a/qa/qa/factory/resource/merge_request.rb
+++ b/qa/qa/factory/resource/merge_request.rb
@@ -12,8 +12,6 @@ module QA
                       :milestone,
                       :labels
 
-        attribute :source_branch
-
         attribute :project do
           Factory::Resource::Project.fabricate! do |resource|
             resource.name = 'project-with-merge-request'
diff --git a/qa/qa/factory/resource/project_imported_from_github.rb b/qa/qa/factory/resource/project_imported_from_github.rb
index f62092ae122..ce20641e6cc 100644
--- a/qa/qa/factory/resource/project_imported_from_github.rb
+++ b/qa/qa/factory/resource/project_imported_from_github.rb
@@ -4,14 +4,13 @@ module QA
   module Factory
     module Resource
       class ProjectImportedFromGithub < Resource::Project
+        attr_accessor :name
         attr_writer :personal_access_token, :github_repository_path
 
         attribute :group do
           Factory::Resource::Group.fabricate!
         end
 
-        attribute :name
-
         def fabricate!
           group.visit!
 
diff --git a/qa/qa/factory/resource/project_milestone.rb b/qa/qa/factory/resource/project_milestone.rb
index cfda58dc103..383f534c12c 100644
--- a/qa/qa/factory/resource/project_milestone.rb
+++ b/qa/qa/factory/resource/project_milestone.rb
@@ -2,14 +2,13 @@ module QA
   module Factory
     module Resource
       class ProjectMilestone < Factory::Base
+        attr_reader :title
         attr_accessor :description
 
         attribute :project do
           Factory::Resource::Project.fabricate!
         end
 
-        attribute :title
-
         def title=(title)
           @title = "#{title}-#{SecureRandom.hex(4)}"
           @description = 'A milestone'
diff --git a/qa/qa/factory/resource/sandbox.rb b/qa/qa/factory/resource/sandbox.rb
index 56bcda9e2f3..a125bac65dd 100644
--- a/qa/qa/factory/resource/sandbox.rb
+++ b/qa/qa/factory/resource/sandbox.rb
@@ -9,7 +9,6 @@ module QA
         attr_reader :path
 
         attribute :id
-        attribute :path
 
         def initialize
           @path = Runtime::Namespace.sandbox_name
diff --git a/qa/qa/factory/resource/ssh_key.rb b/qa/qa/factory/resource/ssh_key.rb
index a48a93fbe65..6f952eda36f 100644
--- a/qa/qa/factory/resource/ssh_key.rb
+++ b/qa/qa/factory/resource/ssh_key.rb
@@ -6,11 +6,9 @@ module QA
       class SSHKey < Factory::Base
         extend Forwardable
 
-        def_delegators :key, :private_key, :public_key, :fingerprint
+        attr_accessor :title
 
-        attribute :private_key
-        attribute :title
-        attribute :fingerprint
+        def_delegators :key, :private_key, :public_key, :fingerprint
 
         def key
           @key ||= Runtime::Key::RSA.new
diff --git a/qa/qa/factory/resource/user.rb b/qa/qa/factory/resource/user.rb
index 6e6f46f7a95..e361face1f0 100644
--- a/qa/qa/factory/resource/user.rb
+++ b/qa/qa/factory/resource/user.rb
@@ -5,6 +5,7 @@ module QA
     module Resource
       class User < Factory::Base
         attr_reader :unique_id
+        attr_writer :username, :password
 
         def initialize
           @unique_id = SecureRandom.hex(8)
@@ -30,11 +31,6 @@ module QA
           defined?(@username) && defined?(@password)
         end
 
-        attribute :name
-        attribute :username
-        attribute :email
-        attribute :password
-
         def fabricate!
           # Don't try to log-out if we're not logged-in
           if Page::Main::Menu.perform { |p| p.has_personal_area?(wait: 0) }
diff --git a/qa/spec/factory/base_spec.rb b/qa/spec/factory/base_spec.rb
index 979dfe2bb26..e9584a27d63 100644
--- a/qa/spec/factory/base_spec.rb
+++ b/qa/spec/factory/base_spec.rb
@@ -69,7 +69,7 @@ describe QA::Factory::Base do
 
     it_behaves_like 'fabrication method', :fabricate_via_api!
 
-    it 'instantiates the factory, calls factory method returns fabrication product' do
+    it 'instantiates the factory, calls factory method returns the resource' do
       expect(factory).to receive(:fabricate_via_api!).and_return(location)
 
       result = subject.fabricate_via_api!(factory: factory, parents: [])
@@ -98,7 +98,7 @@ describe QA::Factory::Base do
       expect(factory).to have_received(:fabricate!).with('something')
     end
 
-    it 'returns fabrication product' do
+    it 'returns fabrication resource' do
       result = subject.fabricate_via_browser_ui!('something', factory: factory, parents: [])
 
       expect(result).to eq(factory)
@@ -138,12 +138,12 @@ describe QA::Factory::Base do
   describe '.attribute' do
     include_context 'simple factory'
 
-    it 'appends new product attribute' do
+    it 'appends new attribute' do
       expect(subject.attributes_names).to eq([:no_block, :test, :web_url])
     end
 
-    context 'when the product attribute is populated via a block' do
-      it 'returns a fabrication product and defines factory attributes as its methods' do
+    context 'when the attribute is populated via a block' do
+      it 'returns value from the block' do
         result = subject.fabricate!(factory: factory)
 
         expect(result).to be_a(described_class)
@@ -151,28 +151,28 @@ describe QA::Factory::Base do
       end
     end
 
-    context 'when the product attribute is populated via the api' do
+    context 'when the attribute is populated via the api' do
       let(:api_resource) { { no_block: 'api' } }
 
       before do
         expect(factory).to receive(:api_resource).and_return(api_resource)
       end
 
-      it 'returns a fabrication product and defines factory attributes as its methods' do
+      it 'returns value from api' do
         result = subject.fabricate!(factory: factory)
 
         expect(result).to be_a(described_class)
         expect(result.no_block).to eq('api')
       end
 
-      context 'when the attribute also has a block in the factory' do
+      context 'when the attribute also has a block' do
         let(:api_resource) { { test: 'api_with_block' } }
 
         before do
           allow(QA::Runtime::Logger).to receive(:info)
         end
 
-        it 'returns the api value and emits an INFO log entry' do
+        it 'returns value from api and emits an INFO log entry' do
           result = subject.fabricate!(factory: factory)
 
           expect(result).to be_a(described_class)
@@ -183,12 +183,12 @@ describe QA::Factory::Base do
       end
     end
 
-    context 'when the product attribute is populated via a factory attribute' do
+    context 'when the attribute is populated via direct assignment' do
       before do
         factory.test = 'value'
       end
 
-      it 'returns a fabrication product and defines factory attributes as its methods' do
+      it 'returns value from the assignment' do
         result = subject.fabricate!(factory: factory)
 
         expect(result).to be_a(described_class)
@@ -200,7 +200,7 @@ describe QA::Factory::Base do
           allow(factory).to receive(:api_resource).and_return({ test: 'api' })
         end
 
-        it 'returns the factory attribute for the product' do
+        it 'returns value from the assignment' do
           result = subject.fabricate!(factory: factory)
 
           expect(result).to be_a(described_class)
@@ -209,12 +209,12 @@ describe QA::Factory::Base do
       end
     end
 
-    context 'when the product attribute has no value' do
+    context 'when the attribute has no value' do
       it 'raises an error because no values could be found' do
         result = subject.fabricate!(factory: factory)
 
         expect { result.no_block }
-          .to raise_error(described_class::NoValueError, "No value was computed for product no_block of factory #{factory.class.name}.")
+          .to raise_error(described_class::NoValueError, "No value was computed for no_block of #{factory.class.name}.")
       end
     end
   end