Refactor core code in common to be a set of POROs with thor wrappers

[eightBitter]

The way you have this now, you are going to have to call get_subjects to test make_index, so in order for tests to pass, you need stable data to test against via live API call (slow, flaky) or to use something like VCR to mock what that result would be. Same for attach_subjects, which invokes make_index This kind of setup leads to hard to write/setup tests that are hard to maintain when something changes. (I have some similar problems in collectionspace-mapper which has this same overall problem without involving Thor) You can greatly simplify the testing by sorta complicating the code. I'm not 100% sure what's going on with your *args here, and undoubtedly this will not straight up work if dropped in, but here's an sketchy idea of what I mean: https://gist.github.com/kspurgin/94c20bfe513304951d6fcc30abc85ff1 It takes the idea of putting your code's actual behavior into Plain Old Ruby Objects (POROs) and mixes in some semi/pseudo dependency injection so the POROs can be called in isolation from one another. I.e. in production, they call on each other like the invoke s you have now, but in testing you can pass in basic test data easily. Then the Thor commands are just dumb wrappers around already tested external code, or your own tested code.

[eightBitter]

Here's the gist in case I lose access:

module Common
  class SubjectGetter
    def self.call
      Aspace_Client.client.use_global_repository
      page = 1
      data = []
      response = Aspace_Client.client.get('subjects', query: {page: page, page_size: 100})
      last_page = response.result['last_page']
      while page <= last_page
        response = Aspace_Client.client.get('subjects', query: {page: page, page_size: 100})
        data << response.result['results']
        page += 1
      end
      data.flatten
    end
  end

  class SubjectIndexMaker
    def self.call(data = SubjectGetter.call)
      index = {}
      data.each do |record|
        index[record['title'].gsub(" -- ", "--")] = record['uri']
      end
      index
    end
  end

  class SubjectAttacher
    def self.call(data, field, index = SubjectIndexMaker.call)
      data.each do |record|
        # sets the variable to empty array if the referenced array is nil; otherwise sets the variable to the array
        # this makes it so that this doesn't override the array if it already exists - it would instead add to the array
        subjects_refs = record["subjects__refs"].nil? ? [] : record["subjects__refs"]
        record[field].each {|entity| subjects_refs << index[entity]}
        record["subjects__refs"] = subjects_refs
      end
      data
    end
  end

  class Subjects < Thor
    desc 'get_subjects', 'retrieve API response of all subject data in ASpace'
    def get_subjects(*args)
      SubjectGetter.call(args)
    end

    desc 'make_index', 'create the following index - "title:uri"'
    def make_index(*args)
      SubjectIndexMaker.call
    end

    desc "attach_subjects", "attach subjects refs to object by matching values from the given field. assumes DATA is an array of hashes, FIELD is a string"
    def attach_subjects(data,field)
      SubjectAttacher.call(data, field)
    end
  end
end

#----- separate spec file

RSpec.describe Common::SubjectIndexMaker do
  describe '.call' do
    it 'returns index' do
      # now it is easy to text what this does with different input
      #   because you can just put test input in the test
      data = 'paste/type data in expected format here'
      expected = 'paste/type data in expected format here'
      expect(Common::SubjectIndexMaker.call(data)).to eq(expected)
    end
  end
end

#----- separate spec file

RSpec.describe Common::SubjectAttacher do
  describe '.call' do
    it 'returns index' do
      # now it is easy to text what this does with different input
      #   because you can just put test input in the test
      data = 'paste/type data in expected format here'
      field = 'fieldname'
      index = 'paste/type data in expected format here'
      expected = 'paste/type data in expected format here'
      expect(Common::SubjectAttacher.call(data, field, index)).to eq(expected)
    end
  end
end