diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..0b0adcd --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,55 @@ +name: CI + +on: + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + ruby-version: ["3.4", "4.0"] + steps: + - uses: actions/checkout@v6 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby-version }} + bundler-cache: true + + - name: Run tests + run: bundle exec rake test + env: + API_URL: http://localhost:8888 + + rubocop: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: "3.4" + bundler-cache: true + + - name: Run Rubocop + run: bundle exec rubocop + + typecheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: "3.4" + bundler-cache: true + + - name: Install RBS collections + run: bundle exec rbs collection install + + - name: Run type checker + run: bundle exec steep check --severity-level=error diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml deleted file mode 100644 index a358807..0000000 --- a/.github/workflows/rubocop.yml +++ /dev/null @@ -1,20 +0,0 @@ -name: Check Rubocop compliance - -on: [push, pull_request] - -jobs: - rubocop: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - ruby-version: "3.4" - - - name: Install dependencies (bundle) - run: bundle install - - - name: Run Rubocop - run: rubocop diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml deleted file mode 100644 index 5338e9c..0000000 --- a/.github/workflows/typecheck.yml +++ /dev/null @@ -1,20 +0,0 @@ -name: typecheck.yml -on: - pull_request: -jobs: - typecheck: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - ruby-version: "3.4" - bundler-cache: true - - - name: Install RBS collections - run: rbs collection install - - - name: Run type checker - run: bundle exec steep check --severity-level=error diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml deleted file mode 100644 index c39ae5a..0000000 --- a/.github/workflows/unit-tests.yml +++ /dev/null @@ -1,25 +0,0 @@ -name: Unit Tests - -on: [push, pull_request] - -jobs: - test: - runs-on: ubuntu-latest - strategy: - matrix: - ruby-version: ["3.4", "4.0"] - steps: - - uses: actions/checkout@v6 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - ruby-version: ${{ matrix.ruby-version }} - - - name: Install dependencies (bundle) - run: bundle install - - - name: Run tests - run: rake test - env: - API_URL: http://localhost:8888 diff --git a/.rubocop.yml b/.rubocop.yml index 457e599..eab5874 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,8 +1,13 @@ +plugins: + - rubocop-minitest + - rubocop-rake + AllCops: TargetRubyVersion: 3.4 Exclude: - config/unicorn.rb - db/** + - vendor/**/* NewCops: enable Layout/LineLength: diff --git a/Gemfile b/Gemfile index bd161c6..cbc1910 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,8 @@ group :development, :test do gem 'ostruct' gem 'rake' gem 'rubocop' + gem 'rubocop-minitest', require: false + gem 'rubocop-rake', require: false gem 'simplecov', require: false gem 'steep', require: false gem 'vcr' diff --git a/Gemfile.lock b/Gemfile.lock index dbaa6c0..7466819 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -89,6 +89,13 @@ GEM rubocop-ast (1.48.0) parser (>= 3.3.7.2) prism (~> 1.4) + rubocop-minitest (0.40.0) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-rake (0.7.1) + lint_roller (~> 1.1) + rubocop (>= 1.72.1) ruby-progressbar (1.13.0) ruby2_keywords (0.0.5) securerandom (0.4.1) @@ -145,6 +152,8 @@ DEPENDENCIES ostruct rake rubocop + rubocop-minitest + rubocop-rake simplecov steep vcr diff --git a/test/data_services_api/dsapi_response_converter_test.rb b/test/data_services_api/dsapi_response_converter_test.rb index af2310f..40ed05c 100644 --- a/test/data_services_api/dsapi_response_converter_test.rb +++ b/test/data_services_api/dsapi_response_converter_test.rb @@ -77,12 +77,14 @@ it 'should convert some SAPINT json to DSAPI format' do actual_response = ppd_resp_conv.send(:to_dsapi_json, 'newBuild', ppd_sapint_items[0]['newBuild']) expected_response = { 'ppd:newBuild' => ppd_dsapi_items[0]['ppd:newBuild'] } + _(actual_response).must_equal expected_response end it 'should convert SAPINT hash to DSAPI format' do actual_response = ppd_resp_conv.send(:to_dsapi_json, 'propertyAddress', 'county' => 'SOMERSET') expected_response = { 'ppd:propertyAddressCounty' => 'SOMERSET' } + _(actual_response).must_equal expected_response end end @@ -341,12 +343,14 @@ it 'should convert some SAPINT json to DSAPI format' do actual_response = ukhpi_resp_conv.send(:to_dsapi_json, 'averagePrice', ukhpi_sapint_items[0]['averagePrice']) expected_response = { 'ukhpi:averagePrice' => ukhpi_dsapi_items[0]['ukhpi:averagePrice'] } + _(actual_response).must_equal expected_response end it 'should convert SAPINT hash to DSAPI format' do actual_response = ukhpi_resp_conv.send(:to_dsapi_json, 'refRegion', '@id' => 'http://landregistry.data.gov.uk/id/region/redcar-and-cleveland') expected_response = { 'ukhpi:refRegion' => { '@id' => 'http://landregistry.data.gov.uk/id/region/redcar-and-cleveland' } } + _(actual_response).must_equal expected_response end end diff --git a/test/data_services_api/query_generator_test.rb b/test/data_services_api/query_generator_test.rb index 714c2af..fa8b738 100644 --- a/test/data_services_api/query_generator_test.rb +++ b/test/data_services_api/query_generator_test.rb @@ -24,6 +24,7 @@ def assert_query(expected, json) it 'should start with an empty query' do pattern = {} query = DataServicesApi::QueryGenerator.new + _(query.to_json).wont_be_nil assert_query(pattern, query.to_json) end @@ -49,6 +50,7 @@ def assert_query(expected, json) it 'should allow sorting to specified' do query = DataServicesApi::QueryGenerator.new + assert_query({ '@sort' => [{ '@up' => 'foo' }] }, query.sort(:up, 'foo').to_json) assert_query({ '@sort' => [{ '@up' => 'foo' }, { '@down' => 'bar' }] }, query.sort(:up, 'foo').sort(:down, 'bar').to_json) @@ -65,6 +67,7 @@ def assert_query(expected, json) it 'should allow an arbitrary relational operator to be added' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction([{ foo: { '@le' => 1000 } }, { foo: { '@ge' => 100 } }]), query.op(:le, :foo, 1000) .op('@ge', :foo, 100) @@ -80,6 +83,7 @@ def assert_query(expected, json) it 'should add a value type when required' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction(foo: { '@ge' => { '@value' => '2014-01-01', '@type' => 'xsd:date' } }), query.op(:ge, :foo, Date.parse('2014-01-01')) .to_json) @@ -87,6 +91,7 @@ def assert_query(expected, json) it 'should allow a text search option to be added' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('@search' => 'foo'), query.search('foo') .to_json) @@ -94,6 +99,7 @@ def assert_query(expected, json) it 'should allow a text search against a specific property to be added' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('@search' => { '@value' => 'foo', '@property' => 'foo:bar' }), query.search_property('foo:bar', 'foo') .to_json) @@ -101,6 +107,7 @@ def assert_query(expected, json) it 'should allow a text search against a specific aspect to be added' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('foo:aspect' => { '@search' => 'foo' }), query.search_aspect('foo:aspect', 'foo') .to_json) @@ -108,6 +115,7 @@ def assert_query(expected, json) it 'should allow a text search against a specific property of an aspect to be added' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('foo:aspect' => { '@search' => { '@value' => 'foo', '@property' => 'foo:bar' } }), query.search_aspect_property('foo:aspect', 'foo:bar', 'foo') .to_json) @@ -115,6 +123,7 @@ def assert_query(expected, json) it 'should allow the limit to be set on a search query' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('foo:aspect' => { '@search' => { '@value' => 'foo', '@property' => 'foo:bar', '@limit' => 997 } }), @@ -129,6 +138,7 @@ def assert_query(expected, json) it 'should allow a simple boolean expression to be added' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('foo:aspect' => { '@oneof' => [{ '@id' => 'foo:bar' }, { '@id' => 'foo:bam' }] }), query.eq_any_uri('foo:aspect', %w[foo:bar foo:bam]) .to_json) @@ -140,6 +150,7 @@ def assert_query(expected, json) it 'should allow a type to be specified for a boolean expression value' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('foo:aspect' => { '@oneof' => [{ '@value' => 'foo:bar', '@type' => 'xsd:coconut' }, { '@value' => 'foo:bam', '@type' => 'xsd:coconut' }] }), query.eq_any_value('foo:aspect', %w[foo:bar foo:bam], type: 'xsd:coconut') @@ -148,6 +159,7 @@ def assert_query(expected, json) it 'should allow a terms to be composed' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction([ { foo: { '@eq' => 'bar' } }, { cat: { '@eq' => 'cow' } }, @@ -159,6 +171,7 @@ def assert_query(expected, json) .to_json) query = DataServicesApi::QueryGenerator.new + assert_query(conjunction([ { 'foo:aspect' => { '@search' => { '@value' => 'foo', '@property' => 'foo:bar' } } }, { 'foo:aspect' => { '@search' => { '@value' => 'fim', '@property' => 'foo:blom' } } } @@ -170,11 +183,13 @@ def assert_query(expected, json) it 'should allow a regex match to be specified' do query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('foo:aspect' => { '@matches' => 'bing.*' }), query.matches('foo:aspect', 'bing.*') .to_json) query = DataServicesApi::QueryGenerator.new + assert_query(conjunction('foo:aspect' => { '@matches' => ['bing.*', 'i'] }), query.matches('foo:aspect', 'bing.*', flags: 'i') .to_json) @@ -182,6 +197,7 @@ def assert_query(expected, json) it 'should allow an overall query limit to be set' do query = DataServicesApi::QueryGenerator.new + assert_query({ '@limit' => 101, '@offset' => 20 }, query.limit(101) .offset(20) @@ -190,6 +206,7 @@ def assert_query(expected, json) it 'should allow a query to be run in count mode' do query = DataServicesApi::QueryGenerator.new + assert_query({ '@count' => true }, query.count_only.to_json) end diff --git a/test/data_services_api/sapint_converter_test.rb b/test/data_services_api/sapint_converter_test.rb index ee2a51b..35966b4 100644 --- a/test/data_services_api/sapint_converter_test.rb +++ b/test/data_services_api/sapint_converter_test.rb @@ -76,6 +76,7 @@ it 'should convert DSAPI and to SAPINT format' do and_json = JSON.parse(dsapi_query)['@and'] + _(sapi_conv.send(:and_list, and_json)).must_equal( sapint_query.map do |key, value| [key, value] unless %w[_count _limit].include?(key) diff --git a/test/data_services_api/service_test.rb b/test/data_services_api/service_test.rb index b7855e3..5e816db 100644 --- a/test/data_services_api/service_test.rb +++ b/test/data_services_api/service_test.rb @@ -33,11 +33,13 @@ def instrument(*args) it 'should return the service URL' do mock_notifier = MockNotifications.new service = DataServicesApi::Service.new(url: 'https://wimbledon.com', instrumenter: mock_notifier) + _(service.url).must_equal('https://wimbledon.com') end it 'should find a dataset by name' do dataset = @service.dataset('ukhpi') + _(dataset.data_api).must_match %r{/landregistry/id/ukhpi} end @@ -52,6 +54,7 @@ def instrument(*args) service = DataServicesApi::Service.new(url: api_url, instrumenter: mock_notifier) json = service.api_get_json("#{api_url}/landregistry/id/ukhpi", { '_limit' => 1 }) + _(json).wont_be_nil _(json['meta']).wont_be_nil end @@ -64,6 +67,7 @@ def instrument(*args) .api_get_json("#{api_url}/landregistry/id/ukhpi", { '_limit' => 1 }) event_names = mock_notifier.instrumentations.map(&:first) + _(event_names).must_include 'response.data_services_api' end @@ -78,6 +82,7 @@ def instrument(*args) end.must_raise event_names = mock_notifier.instrumentations.map(&:first) + _(event_names).must_include 'connection_failure.data_services_api' end @@ -93,6 +98,7 @@ def instrument(*args) _(error.status).must_equal 404 _, payload = mock_notifier.instrumentations.find { |n, _| n == 'service_exception.data_services_api' } + _(payload).wont_be_nil _(payload[:status]).must_equal 404 _(payload[:query_string]).must_equal '_limit=1' @@ -106,6 +112,7 @@ def instrument(*args) .api_get_json("#{api_url}/landregistry/id/ukhpi", { '_limit' => 1 }) _, payload = mock_notifier.instrumentations.find { |name, _| name == 'response.data_services_api' } + _(payload).wont_be_nil _(payload[:response].body['items']).wont_be_nil end @@ -118,6 +125,7 @@ def instrument(*args) .api_get_json("#{api_url}/landregistry/id/ukhpi", { '_limit' => 1 }) _, payload = mock_notifier.instrumentations.find { |name, _| name == 'response.data_services_api' } + _(payload).wont_be_nil _(payload[:duration]).must_be :>, 0 end @@ -130,6 +138,7 @@ def instrument(*args) .api_get_json("#{api_url}/landregistry/id/ukhpi", { '_limit' => 1 }) _, payload = mock_notifier.instrumentations.find { |name, _| name == 'request.data_services_api' } + _(payload).wont_be_nil _(payload[:path]).must_equal '/landregistry/id/ukhpi' _(payload[:query_string]).must_equal '_limit=1' @@ -146,6 +155,7 @@ def instrument(*args) end.must_raise event_names = mock_notifier.instrumentations.map(&:first) + _(event_names).must_include 'request.data_services_api' end @@ -161,6 +171,7 @@ def instrument(*args) end.must_raise retries = mock_notifier.instrumentations.select { |entry| entry.first == 'retry.data_services_api' } + _(retries.size).must_equal 2 _(retries.map { |_, payload| payload[:retry_count] }).must_equal [1, 2] end diff --git a/test/data_services_api/value_test.rb b/test/data_services_api/value_test.rb index d7600ca..0831e41 100644 --- a/test/data_services_api/value_test.rb +++ b/test/data_services_api/value_test.rb @@ -17,6 +17,7 @@ it 'should specify a URI' do v1 = v.with_uri('http://foo/bar') + _(v1.size).must_equal 1 _(v1['@id']).must_equal 'http://foo/bar' _(v1.uri).must_equal 'http://foo/bar' @@ -24,12 +25,14 @@ it 'should have a factory shortcut for creating a URI value' do v = DataServicesApi::Value.uri('http://fubar.com') + _(v.size).must_equal 1 _(v['@id']).must_equal 'http://fubar.com' end it 'should specify type and value' do v1 = v.with_typed_value('foo', 'http://fakexsd.org/bar') + _(v1.size).must_equal 2 _(v1['@value']).must_equal 'foo' @@ -41,6 +44,7 @@ it 'should specify a year and month value' do v1 = v.with_year_month(2016, 2) + _(v1.type).must_equal 'http://www.w3.org/2001/XMLSchema#gYearMonth' _(v1.value).must_equal '2016-02' end