[openstreetmap/openstreetmap-website] Replace trace-related fixtures with factories. (#1347)

Andy Allan notifications at github.com
Tue Nov 1 11:07:19 UTC 2016

So the basis of the stubbing is that we need to test against specific gpx files that are found in the `test/traces/` directory. For example, test_data_compressed needs to use the trace data from `test/traces/4.gpx`.

The Trace objects access the filesystem directly, based on their own object id's. We can't control the ids via factorygirl, and they aren't using any other attributes that we could control when creating the objects. So we need to find something to stub, so for a particular test we can say "use this specific gpx file".

app/models/trace.rb [shells out repeatedly](https://github.com/openstreetmap/openstreetmap-website/blob/269888802c999aa8568ba5ae8c7dad99ecfc2aef/app/models/trace.rb#L219) while reading xml files. We could stub out all of the system and backtick calls to make sure the appropriate file is used, or since each system/backtick uses `trace_name` I chose to stub that out instead.

Similarly for other things like testing icon_picture and large_picture methods - we could stub out the File.new calls, or stub the method that generates the paths. I chose the latter.

Finally, there is an additional twist for some of the calls, where I've used stub_any_instance. This is where the controller method being tested uses `Trace.find` to fetch the objects. We can create a test object and stub the relevant parts, but a second trace object is created when the controller re-fetches it from the database and it too needs stubbing. stub_any_instance means the same stub is applied both to the object that we create, and also to the second object created within the controller. I've only used that where necessary hence why some stubbing is different.

A completely different approach would be to create the test instances, and then copy around the necessary gpx files within the test directory and rename the files to give them the ids assigned by factorygirl. But that seems even worse to me. I'd consider using FakeFS or similar to mock out the filesystem calls, but I doubt that would work with the stuff we're doing backticking out to /usr/bin/file.

I'm happy to refactor if there's a preferred way to do things!

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20161101/dcb98923/attachment.html>

More information about the rails-dev mailing list