-
-
Notifications
You must be signed in to change notification settings - Fork 874
Swiftype Middleman Plugin #2
Comments
@trek Is it acceptable to maintain the field structure in middleman-swiftype, which includes |
If the value of |
Cool, I've got this. |
@Jberlinsky @trek I actually took a stab at this today, and got to the point that a proper |
@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. |
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) |
@bsclifton turns out we also need the body to be |
@trek anything left on this? I think everything is covered |
@bsclifton I think we're good to move forward, but most of the ☑️ still need doing before I'd consider this closeable. |
@tundal45 do you want to take a look at getting the final checkboxes checked? I know @bsclifton is taking a small break |
@trek I am looking into it right now. I will ping you with (m)any questions I may have. |
@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 |
(currently all the logic lives in the Thor sub command) |
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. |
I can start working on this today. |
To recap current thinking: We should migrate the swiftype middleman plugin two three bits:
|
@tundal45 if you don't mind, I can grab this one- I'm pretty familiar with it and have already started making some changes: Should be able to finish this within the next two coding sessions I have 😄 |
@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? :) |
@bsclifton I should have some time this weekend to take a look at this. |
@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! |
Progress made; Submitted LeonB/middleman-swiftype#6 To update the checklist, this item was closed a while back: This PR when accepted will update this item: 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 |
@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. |
Converting the tests to RSpec is always an option too 👍 |
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? |
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 With RSpec, I'd think you'd be good to go unit testing the new helper class: This is a fairly small project, so any testing to increase our confidence would be awesome for both us and the original repo owner :) |
When I run |
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 |
Just cloned to a Nitrous box and didn't get those errors, so must be my machine. I get a new one though: |
Perfect, that means it's working! 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 |
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 For now, I'm trying to convert things over to RSpec. Here's my first pass:
but I get
Any suggestions here? I'm sorry if I'm proving to be a bit useless :( |
I went ahead and just installed the gem on my system with:
Although that doesn't really feel in the spirit of having tests that can run in isolation. Now I'm getting:
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:
But this doesn't seem to change anything. |
@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: |
Okay, I've got a working spec: https://github.com/michaelrkn/middleman-swiftype/commit/1bcf6a8f6e82189f57497bf51eb441269675c599 It's not good, but it works. Problems:
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. |
@michaelrkn thanks for hanging in there- the spec can be stripped even further down: 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 @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 |
The trouble with removing
is that if you don't have the gem already installed, the spec will fail with
|
I just figured out why the stub doesn't work. I changed my test:
And then changed and rebuilt the gem:
And when I run the test, there's no
When you run something from the command line (or in our case, through 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. |
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: |
I'm very close! Hopefully will have something to push up soon. |
Okay, I need to take a break, but here's where I'm at:
The And stubs work! Instead of using
At the moment, this doesn't do anything, because it doesn't find So, to do:
I could use help or at least guidance on all of these :) |
Got the first part!
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. |
Awesome, great job! :) Some tests I can think of, off the top of my head
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 |
Done! PR opened. |
Looks good. We'll just open new issues if they come up |
We need a Swiftype middleman plugin. https://github.com/LeonB/middleman-swiftype publishes directly to Switfype, but our needs are different because publication occurs as a separate step.
A PR that addresses this issue will
swiftype:generate
command that outputs asearch.json
filesearch.json
file goes into thebuild
directory onmiddleman build
andmiddleman server
search.json
fileThe
search.json
must include adocuments
key with an array of the following data:Additionally:
The text was updated successfully, but these errors were encountered: