Swiftype Middleman Plugin

[emberjs]

Swiftype Middleman Plugin

[Jberlinsky]

@trek Is it acceptable to maintain the field structure in middleman-swiftype, which includes title, url, body, info, and conditionally sections and image?

[trek]

If the value of url is the relative path from root (and doesn't include the domain), then yes, that's fine.

[Jberlinsky]

Cool, I've got this.

[dstaley]

@Jberlinsky @trek I actually took a stab at this today, and got to the point that a proper search.json document is generated on both middleman build and middleman swiftype. It's still a WIP (doesn't upload to Swiftype yet), but it covers the majority of the functionality desired in this issue.

[Jberlinsky]

@dstaley I've got this pretty much done -- just have to finish mocking out the HTTP requests for testing. Should have a PR up momentarily.

[dstaley]

@Jberlinsky 👍

[Jberlinsky]

@trek @dstaley See LeonB/middleman-swiftype#3

[bsclifton]

Should be good to go! Made changes here LeonB/middleman-swiftype#4

And #7 makes use of it (from my fork, will update if/when its accepted)

[trek]

@bsclifton turns out we also need the body to be "type": "string" for it to have search term highlights.

[bsclifton]

@trek anything left on this? I think everything is covered

[trek]

@bsclifton I think we're good to move forward, but most of the ☑️ still need doing before I'd consider this closeable.

See LeonB/middleman-swiftype#4 (comment)

[trek]

@tundal45 do you want to take a look at getting the final checkboxes checked? I know @bsclifton is taking a small break

[tundal45]

@trek I am looking into it right now. I will ping you with (m)any questions I may have.

[bsclifton]

@tundal45 it shouldn't be too hard; the extension.rb file currently only listens for after :config event. You can add an after :build hook. The challenge then would be refactoring the code so the same code can be used in the Thor command and also the new build hook

[bsclifton]

(currently all the logic lives in the Thor sub command)

[trek]

The big thing is that we, so far, have regularly rebuilt the docs for 1.10.0 as we find bugs and I'd like to make sure that the search.json ends up in snapshots directory https://github.com/emberjs/guides.emberjs.com/tree/master/snapshots/v1.10.0

The extra step ensure that it doesn't.

[tundal45]

I can start working on this today.

[trek]

To recap current thinking:

We should migrate the swiftype middleman plugin two three bits:

1. a helper module that generates a data structure representing the searchable content
2. a subcommand to build middleman build:swiftype that converts this data structure from noº 1 into JSON and emits a file (search.json) as part of the build process
3. a command middleman swiftype that converts the data structure from noº 1into an HTTP request to the Swiftype API (which matches the original behavior of the plugin before we started messing with it)

[bsclifton]

@tundal45 if you don't mind, I can grab this one- I'm pretty familiar with it and have already started making some changes:
https://github.com/bsclifton/middleman-swiftype/commit/51731f46ded2bd0221905b645f8f1fc3ad409476

Should be able to finish this within the next two coding sessions I have 😄

[bsclifton]

@tundal45 I made decent progress, but am having trouble testing w/ Cucumber since it hits the live server (I tried stubbing, monkeypatching, refining, etc with no luck). @trek said you were able to figure out how to get testing done w/out hitting the server. Did you want to work together on this? :)

[tundal45]

@bsclifton I should have some time this weekend to take a look at this.

[bsclifton]

@tundal45 I got the code working pretty nicely- you can fork off https://github.com/bsclifton/middleman-swiftype

The test cases are the part I'm having problem with :( Any help is much appreciated!

[bsclifton]

Progress made; Submitted LeonB/middleman-swiftype#6

To update the checklist, this item was closed a while back: Provide a swiftype:generate command that outputs a search.json file

This PR when accepted will update this item: search.json file goes into the build directory on middleman build and middleman server

Need help w/ the testing. The tests are cucumber, which I suck at (I'm more familiar with RSpec). Could definitely use an assist here

[michaelrkn]

@bsclifton I'll see if I can sort anything out with Cucumber. I'm more familiar with RSpec, too, but I've got some time to take a crack at it.

[bsclifton]

Converting the tests to RSpec is always an option too 👍

[michaelrkn]

Yeah, I'll just do that unless there's a reason to use Cucumber. I see two specs in there currently; are those the only things that need tests at the moment? If there's more that needs testing, is there a place to see what, or a good way to figure it out?

[bsclifton]

The cucumber tests might be OK to leave there since they do test that running the given commands fires the plugin. We could just then add a third test which does middleman build and then expects the search.json to be there (this is the new functionality).

With RSpec, I'd think you'd be good to go unit testing the new helper class:
https://github.com/bsclifton/middleman-swiftype/blob/master/lib/middleman-swiftype-helper.rb

This is a fairly small project, so any testing to increase our confidence would be awesome for both us and the original repo owner :)

[michaelrkn]

When I run $ cucumber from the project root, I get unable to find ./middleman-swiftype.gemspec for gem middleman-swiftype (Gem::GemNotFoundException). Any suggestions?

[bsclifton]

Hate saying this, but works for me? Here's what I did (running on Cent OS 6.6):

git clone https://github.com/bsclifton/middleman-swiftype.git
cd middleman-swiftype/
bundle install
bundle exec cucumber

[michaelrkn]

Just cloned to a Nitrous box and didn't get those errors, so must be my machine.

I get a new one though: /home/action/.gem/ruby/2.1.3/gems/swiftype-1.1.0/lib/swiftype/request.rb:55:inhandle_errors': Swiftype::InvalidCredentials (Swiftype::InvalidCredentials)`. Where should I set those credentials? Should I make a Swiftype account of my own for testing?

[bsclifton]

Perfect, that means it's working!
We could setup a test account- the config it's using right now lives here

I tried to stub the test out (so it doesn't actually call out to Swiftype for real). Unfortunately, I wasn't able to figure it out. You can create step definitions where it sounds like Cucumber can create allow/expects using RSpec syntax. However, this didn't work for me.

I tried both monkey patching and using refinements to attempt to no-op the method in question (which is throwing the error you're getting after making a real call to Swiftype), but it didn't work
https://github.com/bsclifton/middleman-swiftype/blob/aa3e439dbd16fdae1827c72d071fa9653896c5b9/features/support/env.rb

[michaelrkn]

Okay, I'm about ready to give up on Cucumber. I've tried everything I can think of, and while it looks like the step definition is getting called (I can see output when I puts something from there), it doesn't look like any stubs I define there are actually getting used in the app. I threw something up on SO to see about getting some help: http://stackoverflow.com/questions/29378634/rspec-stub-not-working-with-cucumber

For now, I'm trying to convert things over to RSpec. Here's my first pass:

require "middleman-swiftype"

describe "generating swiftype manifest files" do
  it "creates a search.json file" do
    swiftype_client_double = double('swiftype_client_double')
    allow(::Swiftype::Client).to receive(:new).and_return(swiftype_client_double)
    allow(swiftype_client_double).to receive(:create_or_update_document)

    `middleman swiftype --only-generate`
    expect(File).to exist("build/search.json")
  end
end

but I get

There's no 'swiftype' command for Middleman. Try 'middleman help' for a list of commands.

Any suggestions here?

I'm sorry if I'm proving to be a bit useless :(

[michaelrkn]

I went ahead and just installed the gem on my system with:

gem build middleman-swiftype.gemspec
gem install middleman-swiftype-0.0.1.gem

Although that doesn't really feel in the spirit of having tests that can run in isolation.

Now I'm getting:

/Users/michael/Code/middleman-swiftype/lib/middleman-swiftype/commands.rb:64:in `print_usage_and_die': undefined local variable or method `settings' for #<Middleman::Cli::Swiftype:0x007f98fc82c340> (NameError)

It seems like Cucumber is somehow setting up this "fixture app", but I don't see how that is being done, or how it helps this error.

I've tried cd'ing to the fixture app in my tests:

system "cd #{Dir.pwd}/fixtures/swiftype-app"
system "middleman swiftype --only-generate"

But this doesn't seem to change anything.

[bsclifton]

@michaelrkn I fixed the above issue w/ https://github.com/bsclifton/middleman-swiftype/commit/8bd5518d51422a0705693af46159c784fb8a0f5d (you can go ahead and sync w/ my branch). I never hit that code path I guess? It was trying to evaluate "#{settings.url}", which wasn't defined (just needed to be escaped, since it's printing example usage).

With RSpec, it should be easy enough to stub out the methods that the code is calling. I put together a quick example based on your code. This example doesn't quite work, but I'm guessing it should be something like this:
https://gist.github.com/bsclifton/3673cdd8950ded628464

[michaelrkn]

Okay, I've got a working spec: https://github.com/michaelrkn/middleman-swiftype/commit/1bcf6a8f6e82189f57497bf51eb441269675c599

It's not good, but it works.

Problems:

• It works even after I've commented the create_or_update_document stub, even with the internet off. I'm worried this is a bad thing but hoping it's a good thing. I'm too crazy from winding through the code so much over the past day to really understand it any more ;)
• Is ending with system "rm #{Dir.pwd}/fixtures/swiftype-app/build/search.json" the best way to clean up after the spec runs, or is there a cleaner way to do it?
• I've added a before :all to setup installation of the gem, and a after :all to remove it. This doesn't feel like the best approach, but testing a gem is new to me.

Once we sort out these issues and feel like we have a good testing approach, it should be pretty simple to get the rest of the integration tests done.

[bsclifton]

@michaelrkn thanks for hanging in there- the spec can be stripped even further down:
https://gist.github.com/bsclifton/13bca52dfb12d60672b6

This works basically the same as running the working Cucumber test (just using RSpec). Still there's the issue of testing regular path (pushing to Swiftype). The reason it works even without the stub is because it's never being used. The fixtures\swiftype-app directory has a config.rb which gets loaded (which activates the plugin with a default set of options)

@Jberlinsky would you be able to help us figure out the Cucumber tests? I'm pulling my hair out figuring out how to mock the create_or_update_document call in this test. You can clone the repo from my fork and run bundle exec cucumber to see the failing test (since it actually reaches out to swiftype). You had wrote the original test so wanted to ask you for help

[michaelrkn]

The trouble with removing

  before :all do
    system "gem build middleman-swiftype.gemspec"
    system "gem install middleman-swiftype-0.0.1.gem"
  end

  after :all do
    system "rm middleman-swiftype-0.0.1.gem"
    system "gem uninstall middleman-swiftype"
  end

is that if you don't have the gem already installed, the spec will fail with

There's no 'swiftype' command for Middleman. Try 'middleman help' for a list of commands.

[michaelrkn]

I just figured out why the stub doesn't work. I changed my test:

it "creates a search.json file" do
    allow(Hash).to receive(:new).and_return('pie')
    Dir.chdir("fixtures/swiftype-app") do
      system "middleman swiftype --only-generate"
      expect(File).to exist("build/search.json")
      system "rm build/search.json"
    end
  end

And then changed and rebuilt the gem:

class MiddlemanSwiftypeHelper
  def initialize(plugin_options)
    @options = plugin_options
    @mm_instance = Middleman::Application.server.inst
    p Hash.new
  end
...

And when I run the test, there's no 'pie':

1 gem installed
{}
Generating search.json...
Finished generating search.json.
...

When you run something from the command line (or in our case, through system), it's being run by a different instance of ruby than the ruby that's running our tests. We're only stubbing the methods in our test, but we can't reach across into the ruby that's being run from the command line.

I'm thinking that instead of running a completely end-to-end test, maybe we can just directly call the method that the command-line command invokes. That way, we don't have to set up and tear down the gem installation, and our stubs will actually work.

What do you think?

I'm going to start playing around with the code to make this happen.

[bsclifton]

That would totally work- basically unit testing it instead of doing an integration test. You can create an instance of the Middleman::Cli::Swiftype object and then execute both the after config and after build code paths :smile:

[michaelrkn]

I'm very close! Hopefully will have something to push up soon.

[michaelrkn]

Okay, I need to take a break, but here's where I'm at:

require "middleman-swiftype"

describe "generating swiftype manifest files" do
  it "creates a search.json file" do
    Dir.chdir("fixtures/swiftype-app") do
      task = Middleman::Cli::Swiftype.new
      task.options = {:'only-generate' => true}
      task.invoke(:swiftype)
      expect(File).to exist("build/search.json")
    end
  end
end

The task.options bit doesn't actually seem to be passing in the options, but other than that, it works.

And stubs work! Instead of using Dir.chdir, you can do:

require "middleman-swiftype"

describe "generating swiftype manifest files" do
  let :default_options do
    OpenStruct.new({
      :api_key => "API_KEY",
      :engine_slug => "middleman",
      :pages_selector => lambda { |p| p.path.match(/\.html/) && p.metadata[:options][:layout] == nil },
      :process_html => lambda { |f| f.search('.//div[@class="linenodiv"]').remove },
      :generate_sections => lambda { |p| (p.metadata[:page]['tags'] ||= []) + (p.metadata[:page]['categories'] ||= []) },
      :generate_info => lambda { |f| f.to_s },
      :generate_image => lambda { |p| "\#{settings.url}\#{p.metadata[:page]['banner']}" if p.metadata[:page]['banner'] }
    })
  end

  before :each do
    allow_any_instance_of(Middleman::Application).to receive_message_chain(:swiftype, :options).and_return(default_options)
  end

  it "creates a search.json file" do
    task = Middleman::Cli::Swiftype.new
    task.options = {:'only-generate' => true}
    task.invoke(:swiftype)
    expect(File).to exist("build/search.json")
  end
end

At the moment, this doesn't do anything, because it doesn't find index.html, so there's nothing to generate. But it might be cleaner than using a fixture app? Not sure.

So, to do:

• Figure out how to pass task options when invoking a Thor command from Ruby.
• Decide whether to use the fixture app or stub everything out.
• If we stub everything, figure out how to stub out the index.html file.

I could use help or at least guidance on all of these :)

[michaelrkn]

Got the first part!

require "middleman-swiftype"

describe "generating swiftype manifest files" do
  it "creates a search.json file" do
    Dir.chdir("fixtures/swiftype-app") do
      task = Middleman::Cli::Swiftype.new
      task.invoke(:swiftype, [], {:'only-generate' => true})
      expect(File).to exist("build/search.json")
      system("rm build/search.json")
    end
  end
end

Pushed up to https://github.com/michaelrkn/middleman-swiftype/commit/554c10f680a4a36040df900510ab64a9e52051a5

I'll start working on the other specs now. I'll start by converting the other Cucumber spec to this approach.

[bsclifton]

Awesome, great job! :)

Some tests I can think of, off the top of my head

1. Making sure the body field is type "string" (important for our use-case; otherwise doesn't highlight)
2. Making sure the search.json is built after the Middleman builder fires. Triggering that should be as easy as doing

builder = Middleman::Cli::Build.new
builder.build

3. Making sure the regular use-case (what the original repo-owner made the plugin for) works; basically that the same task, without the params, calls the swiftype create_or_update_document
4. Documenting how to run the tests in the README.md
5. Removing the cucumber tests

After that, I think we're covered! Thanks for jumping in and really owning this :)

My fork still has a PR open with the owner; feel free to open a PR against my fork and I'll accept it

[michaelrkn]

Done! PR opened.

[bsclifton]

@trek this issue should be closed when #194 is merged; can you please re-check against the original checklist above? 😄

[trek]

Looks good. We'll just open new issues if they come up