Skip to content

Gerardo's solution #314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Gerardo's solution #314

wants to merge 1 commit into from

Conversation

gerardo-m
Copy link

Solution

I tried to keep the solution in its simplest form. It's basically a file and a test file. I added the
Gemfile at the end for convenience.

The instructions only said that it must work for the same kind of carrousel, which is very specific
since other image searches show a different type of carrousel.

My focus was to reproduce the same result as the expected-array.json file and without making any
http requests. And of course the test suite which I want to elaborate a little:

  • I think every test should have only 1 expect.
  • Every test must have a clear goal, with only 1 scenario.

I think it is better appreciated in the following section:

let(:artwork_with_empty_extensions) { result["artworks"].select{|a| a["name"]=="Sunflowers"}.first}
describe 'extensions' do
    it "must be an Array, if present" do
        expect(artwork["extensions"]).to be_an Array
    end

    it "can't contain empty Strings" do
        expect(artwork["extensions"]).not_to include("")
    end

    it "should not be present if value is empty" do
        expect(artwork_with_empty_extensions).not_to include("extensions")
    end
end

There was an scenario where the extensions are not set, in the expected results the extensions were
not present, so I provided a specific scenario to the test so it can be properly tested.

To test

Just run

bundle install
ruby test.rb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant