Merge pull request #37 from localhots/fix-tests-2024

Fix tests and setup GitHub Actions to run tests automatically

.github/workflows/test.yml

@@ -0,0 +1,26 @@
+# Adopted from
+# https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-ruby
+name: Ruby CI
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        ruby-version: ['3.3']
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Ruby ${{ matrix.ruby-version }}
+        uses: ruby/[email protected]
+        with:
+          ruby-version: ${{ matrix.ruby-version }}
+      - name: Install dependencies
+        run: bundle install
+      - name: Run tests
+        run: bundle exec rspec

lib/musicbrainz/client_modules/caching_proxy.rb

@@ -19,28 +19,42 @@ def get_contents(url)
         response = { body: nil, status: 500 }
 
         if File.exist?(file_path)
-          response = {
-            body: File.open(file_path, 'rb').gets,
-            status: 200
-          }
+          get_cached_contents(file_path)
         else
-          response = super
+          response = get_live_contents(file_path, url)
           if response[:status] == 200
-            FileUtils.mkpath(dir_path)
-            File.open(file_path, 'wb') do |f|
-              f.puts(response[:body])
-              f.chmod(0755)
-              f.close
-            end
+            cache_contents!(file_path, response[:body])
           end
+          response
         end
-
-        response
       end
 
       def caching?
         MusicBrainz.config.perform_caching
       end
+
+    private
+
+      def get_cached_contents(file_path)
+        {
+          body: File.open(file_path, 'rb').gets,
+          status: 200
+        }
+      end
+
+      def get_live_contents(file_path, url)
+        http.get(url)
+      end
+
+      def cache_contents!(file_path, body)
+        dir_path = File.dirname(file_path)
+        FileUtils.mkpath(dir_path)
+        File.open(file_path, 'wb') do |f|
+          f.puts(body)
+          f.chmod(0755)
+          f.close
+        end
+      end
     end
   end
 end

spec/bindings/recording_search_spec.rb

@@ -2,12 +2,45 @@
 
 describe MusicBrainz::Bindings::RecordingSearch do
   describe '.parse' do
+    let(:response) {
+      <<-XML
+        <metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#" xmlns:ext="http://musicbrainz.org/ns/ext#-2.0">
+          <recording-list count="1" offset="0">
+            <recording ext:score="100" id="0b382a13-32f0-4743-9248-ba5536a6115e">
+              <title>King Fred</title>
+              <artist-credit>
+                <name-credit>
+                  <artist id="f52f7a92-d495-4d32-89e7-8b1e5b8541c8">
+                    <name>Too Much Joy</name>
+                  </artist>
+                </name-credit>
+              </artist-credit>
+              <release-list>
+                <release id="8442e42b-c40a-4817-89a0-dbe663c94d2d">
+                  <title>Green Eggs and Crack</title>
+                </release>
+              </release-list>
+            </recording>
+          </recording-list>
+        </metadata>
+      XML
+    }
+    let(:xml) {
+      Nokogiri::XML.parse(response).remove_namespaces!
+    }
+    let(:metadata) {
+      described_class.parse(xml.xpath('/metadata'))
+    }
+
     it "gets correct Recording data" do
-			response = '<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#" xmlns:ext="http://musicbrainz.org/ns/ext#-2.0"><recording-list offset="0" count="1"><recording id="0b382a13-32f0-4743-9248-ba5536a6115e" ext:score="100"><title>King Fred</title><artist-credit><name-credit><artist id="f52f7a92-d495-4d32-89e7-8b1e5b8541c8"><name>Too Much Joy</name></artist></name-credit></artist-credit><release-list><release id="8442e42b-c40a-4817-89a0-dbe663c94d2d"><title>Green Eggs and Crack</title></release></release-list></recording></recording-list></metadata>'
-      expect(described_class.parse(Nokogiri::XML.parse(response).remove_namespaces!.xpath('/metadata'))).to eq [
+      expect(metadata).to eq [
         {
-          id: '0b382a13-32f0-4743-9248-ba5536a6115e', mbid: '0b382a13-32f0-4743-9248-ba5536a6115e',
-          title: 'King Fred', artist: 'Too Much Joy', releases: ['Green Eggs and Crack'], score: 100
+          id: '0b382a13-32f0-4743-9248-ba5536a6115e',
+          mbid: '0b382a13-32f0-4743-9248-ba5536a6115e',
+          title: 'King Fred',
+          artist: 'Too Much Joy',
+          releases: ['Green Eggs and Crack'],
+          score: 100,
         }
       ]
     end

spec/bindings/relations_spec.rb

@@ -5,34 +5,55 @@
     describe 'attributes' do
       describe 'urls' do
         context '1 url for relation type' do
-          it 'returns a string' do
-            xml = Nokogiri::XML.parse(
-              %Q{<artist><relation-list target-type="url">
-                <relation type-id="99429741-f3f6-484b-84f8-23af51991770" type="social network">
-                  <target id="4f4068cb-7001-47a3-a2fe-9146eb6b5d16">https://plus.google.com/+Madonna</target>
-                </relation>
-              </relation-list></artist>}
-            )
+          let(:response) {
+            <<-XML
+              <artist>
+                <relation-list target-type="url">
+                  <relation type="social network" type-id="99429741-f3f6-484b-84f8-23af51991770">
+                    <target id="4f4068cb-7001-47a3-a2fe-9146eb6b5d16">https://plus.google.com/+Madonna</target>
+                  </relation>
+                </relation-list>
+              </artist>
+            XML
+          }
+          let(:xml) {
+            Nokogiri::XML.parse(response)
+          }
+          let(:artist) {
+            described_class.parse(xml.xpath('./artist'))
+          }
 
-            expect(described_class.parse(xml.xpath('./artist'))[:urls][:social_network]).to eq 'https://plus.google.com/+Madonna'
+          it 'returns a string' do
+            expect(artist[:urls][:social_network]).to eq 'https://plus.google.com/+Madonna'
           end
         end
 
         context 'multiple urls for relation types' do
-          it 'returns an array' do
-            xml = Nokogiri::XML.parse(
-              %Q{<artist><relation-list target-type="url">
-                <relation type-id="99429741-f3f6-484b-84f8-23af51991770" type="social network">
-                  <target id="4f4068cb-7001-47a3-a2fe-9146eb6b5d16">https://plus.google.com/+Madonna</target>
-                </relation>
-                <relation type-id="99429741-f3f6-484b-84f8-23af51991770" type="social network">
-                  <target id="1dc9e14d-ebfb-448c-a005-e3481d320595">https://www.facebook.com/madonna</target>
-                </relation>
-              </relation-list></artist>}
-            )
+          let(:response) {
+            <<-XML
+              <artist>
+                <relation-list target-type="url">
+                  <relation type="social network" type-id="99429741-f3f6-484b-84f8-23af51991770">
+                    <target id="4f4068cb-7001-47a3-a2fe-9146eb6b5d16">https://plus.google.com/+Madonna</target>
+                  </relation>
+                  <relation type="social network" type-id="99429741-f3f6-484b-84f8-23af51991770">
+                    <target id="1dc9e14d-ebfb-448c-a005-e3481d320595">https://www.facebook.com/madonna</target>
+                  </relation>
+                </relation-list>
+              </artist>
+            XML
+          }
+          let(:xml) {
+            Nokogiri::XML.parse(response)
+          }
+          let(:artist) {
+            described_class.parse(xml.xpath('./artist'))
+          }
 
-            expect(described_class.parse(xml.xpath('./artist'))[:urls][:social_network]).to eq [
-              'https://plus.google.com/+Madonna', 'https://www.facebook.com/madonna'
+          it 'returns an array' do
+            expect(artist[:urls][:social_network]).to eq [
+              'https://plus.google.com/+Madonna',
+              'https://www.facebook.com/madonna',
             ]
           end
         end

spec/bindings/release_group_search_spec.rb

@@ -3,19 +3,33 @@
 describe MusicBrainz::Bindings::ReleaseGroupSearch do
   describe '.parse' do
     let(:response) {
-      '<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#" xmlns:ext="http://musicbrainz.org/ns/ext#-2.0"><release-group-list><release-group id="246bc928-2dc8-35ba-80ee-7a0079de1632" type="Single" ext:score="100"><title>Empire</title></release-group></release-group-list></metadata>'
+      <<-XML
+        <metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#" xmlns:ext="http://musicbrainz.org/ns/ext#-2.0">
+          <release-group-list>
+            <release-group ext:score="100" id="246bc928-2dc8-35ba-80ee-7a0079de1632" type="Single">
+              <title>Empire</title>
+            </release-group>
+          </release-group-list>
+        </metadata>
+      XML
+    }
+    let(:xml) {
+      Nokogiri::XML.parse(response)
     }
     let(:metadata) {
-      described_class.parse(Nokogiri::XML.parse(response).remove_namespaces!.xpath('/metadata'))
+      described_class.parse(xml.remove_namespaces!.xpath('/metadata'))
     }
 
     it "gets correct release group data" do
-      expect(metadata).to eq([
+      expect(metadata).to eq [
         {
-          id: '246bc928-2dc8-35ba-80ee-7a0079de1632', mbid: '246bc928-2dc8-35ba-80ee-7a0079de1632',
-          title: 'Empire', type: 'Single', score: 100
+          id: '246bc928-2dc8-35ba-80ee-7a0079de1632',
+          mbid: '246bc928-2dc8-35ba-80ee-7a0079de1632',
+          title: 'Empire',
+          type: 'Single',
+          score: 100,
         }
-      ])
+      ]
     end
   end
 end

spec/bindings/release_spec.rb

@@ -5,34 +5,112 @@
     describe 'attributes' do
       describe 'format' do
         context 'single cd' do
+          let(:response) {
+            <<-XML
+              <release>
+                <medium-list count="1">
+                  <medium>
+                    <format>CD</format>
+                  </medium>
+                </medium-list>
+              </release>
+            XML
+          }
+          let(:xml) {
+            Nokogiri::XML.parse(response)
+          }
+          let(:release) {
+            described_class.parse(xml)
+          }
+
           it 'returns CD' do
-            response = '<release><medium-list count="1"><medium><format>CD</format></medium></medium-list></release>'
-            xml = Nokogiri::XML.parse(response)
-            expect(described_class.parse(xml)[:format]).to eq 'CD'
+            expect(release[:format]).to eq 'CD'
           end
         end
 
         context 'multiple cds' do
+          let(:response) {
+            <<-XML
+              <release>
+                <medium-list count="2">
+                  <medium>
+                    <format>CD</format>
+                    <track-list count="11"/>
+                  </medium>
+                  <medium>
+                    <title>bonus disc</title>
+                    <format>CD</format>
+                  </medium>
+                </medium-list>
+              </release>
+            XML
+          }
+          let(:xml) {
+            Nokogiri::XML.parse(response)
+          }
+          let(:release) {
+            described_class.parse(xml)
+          }
+
           it 'returns 2xCD' do
-            response = '<release><medium-list count="2"><medium><format>CD</format><track-list count="11" /></medium><medium><title>bonus disc</title><format>CD</format></medium></medium-list></release>'
-            xml = Nokogiri::XML.parse(response)
-            expect(described_class.parse(xml)[:format]).to eq '2xCD'
+            expect(release[:format]).to eq '2xCD'
           end
         end
 
         context 'different formats' do
+          let(:response) {
+            <<-XML
+              <release>
+                <medium-list count="2">
+                  <medium>
+                    <format>DVD</format>
+                  </medium>
+                  <medium>
+                    <format>CD</format>
+                  </medium>
+                </medium-list>
+              </release>
+            XML
+          }
+          let(:xml) {
+            Nokogiri::XML.parse(response)
+          }
+          let(:release) {
+            described_class.parse(xml)
+          }
+
           it 'returns DVD + CD' do
-            response = '<release><medium-list count="2"><medium><format>DVD</format></medium><medium><format>CD</format></medium></medium-list></release>'
-            xml = Nokogiri::XML.parse(response)
-            expect(described_class.parse(xml)[:format]).to eq 'DVD + CD'
+            expect(release[:format]).to eq 'DVD + CD'
           end
         end
 
         context 'different formats plus multiple mediums with same format' do
+          let(:response) {
+            <<-XML
+              <release>
+                <medium-list count="2">
+                  <medium>
+                    <format>CD</format>
+                  </medium>
+                  <medium>
+                    <format>CD</format>
+                  </medium>
+                  <medium>
+                    <format>DVD</format>
+                  </medium>
+                </medium-list>
+              </release>
+            XML
+          }
+          let(:xml) {
+            Nokogiri::XML.parse(response)
+          }
+          let(:release) {
+            described_class.parse(xml)
+          }
+
           it 'returns 2xCD + DVD' do
-            response = '<release><medium-list count="2"><medium><format>CD</format></medium><medium><format>CD</format></medium><medium><format>DVD</format></medium></medium-list></release>'
-            xml = Nokogiri::XML.parse(response)
-            expect(described_class.parse(xml)[:format]).to eq '2xCD + DVD'
+            expect(release[:format]).to eq '2xCD + DVD'
           end
         end
       end

spec/client_modules/cache_spec.rb

@@ -5,8 +5,7 @@
   let(:test_mbid){ "69b39eab-6577-46a4-a9f5-817839092033" }
   let(:tmp_cache_path){ File.join(File.dirname(__FILE__), '..', '..', 'tmp', 'cache_module_spec_cache') }
   let(:test_cache_file){ "#{tmp_cache_path}/03/48/ec/6c2bee685d9a96f95ed46378f624714e7a4650b0d44c1a8eee5bac2480.xml" }
-  let(:test_response_file){ File.join(File.dirname(__FILE__), "../fixtures/kasabian.xml") }
-  let(:test_response){ File.open(test_response_file).read }
+  let(:test_response){ read_fixture('artist/find_69b39eab-6577-46a4-a9f5-817839092033.xml') }
 
   before(:all) do
     MusicBrainz.config.cache_path = File.join(File.dirname(__FILE__), '..', '..', 'tmp', 'cache_module_spec_cache')
@@ -28,8 +27,9 @@
       expect(File).to_not exist(test_cache_file)
 
       # Stubbing
-      allow(MusicBrainz.client.http).to receive(:get).and_return(OpenStruct.new(status: 200, body: test_response))
-      expect(MusicBrainz.client.http).to receive(:get).once
+      allow(MusicBrainz.client).to receive(:get_live_contents)
+        .and_return({status: 200, body: test_response})
+      expect(MusicBrainz.client).to receive(:get_live_contents).once
 
       2.times do
         artist = MusicBrainz::Artist.find(test_mbid)