Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Insert example code in original headers #7569

Merged
merged 21 commits into from
Jan 4, 2017

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Jan 2, 2017

The script for inserting example code into documentation comments in headers (added in #7337) is no longer run as a build phase. Instead, after you add the Swift code to the test method and add

```swift
```

and any front matter to the original, checked-in headers, you invoke make darwin-update-examples manually. The script inserts the examples into the original checked-in headers instead of the headers in build products. You no longer have to invoke make iframework before invoking make idocument.

You can invoke the make rule as many times as you like. The headers will always be updated with the latest code from the test methods, even if the code block has already been filled in. This makes darwin-update-examples a lot like style-code-darwin, except that we still manage the rest of the header (including parts of the comment adjacent to the example) in the header itself. Also, there’s no reminder at the top of every header to invoke make darwin-update-examples. If folks tend to forget to update the examples via the test methods, we could make the script’s dry run output more parsable and run it from within a unit test.

The script updates code blocks in all public headers for both the iOS SDK and the macOS SDK. In the event that a header is used by both SDKs and the test method has a conditional compilation block, the iOS version takes precedence over the macOS version. Thanks to type inference in Swift, there’s only one place where conditional compilation is required for passing tests.

In order to reliably associate documentation comments with their symbols, the script calls out to sourcekitten.

Fixes #7565.

@1ec5 1ec5 added documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling tests labels Jan 2, 2017
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Jan 2, 2017
@1ec5 1ec5 self-assigned this Jan 2, 2017
@1ec5 1ec5 requested review from ericrwolfe and friedbunny January 2, 2017 10:03
@mention-bot
Copy link

@1ec5, thanks for your PR! By analyzing this pull request, we identified @boundsj, @incanus and @friedbunny to be potential reviewers.

@friedbunny
Copy link
Contributor

friedbunny commented Jan 2, 2017

This is still failing after 3759267:

Test Suite 'MGLDocumentationExampleTests' started at 2017-01-02 03:23:44.055
Test Case '-[test.MGLDocumentationExampleTests testMGLCircleStyleLayer]' started.
libc++abi.dylib: terminating with uncaught exception of type std::runtime_error: You must
  provide a Mapbox API access token for Mapbox tile sources

@friedbunny
Copy link
Contributor

friedbunny commented Jan 2, 2017

@1ec5 I pushed the SourceKitten check/install change in 27092f3 that we discussed.

execFileSync() will throw if the external process returns a non-zero exit code, so I think this try-catch block is ultimately the cleanest way.

Copy link
Contributor

@ericrwolfe ericrwolfe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 🚢

Copy link
Contributor

@friedbunny friedbunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and I really appreciate the changes, but we should address the failing test.

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 4, 2017

I’m able to reproduce the test failure only after running the entire test bundle cold – that is, after quitting the simulator. Although we’re careful to initialize the map view with a local style URL, somehow the map ends up loading a Mapbox-hosted source:

#0	0x000000010e1fdf74 in mbgl::DefaultFileSource::request(mbgl::Resource const&, std::__1::function<void (mbgl::Response)>)::DefaultFileRequest::DefaultFileRequest(mbgl::Resource, std::__1::function<void (mbgl::Response)>, mbgl::util::Thread<mbgl::DefaultFileSource::Impl>&) at /Users/mxn/hub/mapbox-gl-native/platform/default/default_file_source.cpp:193
#1	0x000000010e1fdf4d in mbgl::DefaultFileSource::request(mbgl::Resource const&, std::__1::function<void (mbgl::Response)>)::DefaultFileRequest::DefaultFileRequest(mbgl::Resource, std::__1::function<void (mbgl::Response)>, mbgl::util::Thread<mbgl::DefaultFileSource::Impl>&) at /Users/mxn/hub/mapbox-gl-native/platform/default/default_file_source.cpp:195
#2	0x000000010e1ea5bb in std::__1::__unique_if<mbgl::DefaultFileSource::request(mbgl::Resource const&, std::__1::function<void (mbgl::Response)>)::DefaultFileRequest>::__unique_single std::__1::make_unique<mbgl::DefaultFileSource::request(mbgl::Resource const&, std::__1::function<void (mbgl::Response)>)::DefaultFileRequest, mbgl::Resource const&, std::__1::function<void (mbgl::Response)>&, mbgl::util::Thread<mbgl::DefaultFileSource::Impl>&>(mbgl::Resource const&&&, std::__1::function<void (mbgl::Response)>&&&, mbgl::util::Thread<mbgl::DefaultFileSource::Impl>&&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:3141
#3	0x000000010e1ea544 in mbgl::DefaultFileSource::request(mbgl::Resource const&, std::__1::function<void (mbgl::Response)>) at /Users/mxn/hub/mapbox-gl-native/platform/default/default_file_source.cpp:211
#4	0x000000010e67749a in mbgl::style::TileSourceImpl::loadDescription(mbgl::FileSource&) at /Users/mxn/hub/mapbox-gl-native/src/mbgl/style/tile_source_impl.cpp:64
#5	0x000000010e652834 in mbgl::style::Style::recalculate(float, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > const&, mbgl::MapMode) at /Users/mxn/hub/mapbox-gl-native/src/mbgl/style/style.cpp:337
#6	0x000000010e3cf55b in mbgl::Map::Impl::update() at /Users/mxn/hub/mapbox-gl-native/src/mbgl/map/map.cpp:254
#7	0x000000010e3de6b8 in mbgl::Map::Impl::Impl(mbgl::Backend&, float, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode, mbgl::GLContextMode, mbgl::ConstrainMode, mbgl::ViewportMode)::$_1::operator()() const at /Users/mxn/hub/mapbox-gl-native/src/mbgl/map/map.cpp:138
#8	0x000000010e3de68d in std::__1::__invoke<mbgl::Map::Impl::Impl(mbgl::Backend&, float, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode, mbgl::GLContextMode, mbgl::ConstrainMode, mbgl::ViewportMode)::$_1&>(decltype(std::__1::forward<mbgl::Map::Impl::Impl(mbgl::Backend&, float, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode, mbgl::GLContextMode, mbgl::ConstrainMode, mbgl::ViewportMode)::$_1&>(fp)(std::__1::forward<>(fp0))), mbgl::Map::Impl::Impl(mbgl::Backend&, float, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode, mbgl::GLContextMode, mbgl::ConstrainMode, mbgl::ViewportMode)::$_1&&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/__functional_base:416
#9	0x000000010e3de67c in _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN4mbgl3Map4ImplC1ERNS3_7BackendEfRNS3_10FileSourceERNS3_9SchedulerENS3_7MapModeENS3_13GLContextModeENS3_13ConstrainModeENS3_12ViewportModeEE3$_1EEEvDpOT_ at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/__functional_base:468
#10	0x000000010e3de5a9 in std::__1::__function::__func<mbgl::Map::Impl::Impl(mbgl::Backend&, float, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode, mbgl::GLContextMode, mbgl::ConstrainMode, mbgl::ViewportMode)::$_1, std::__1::allocator<mbgl::Map::Impl::Impl(mbgl::Backend&, float, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode, mbgl::GLContextMode, mbgl::ConstrainMode, mbgl::ViewportMode)::$_1>, void ()>::operator()() at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/functional:1437
#11	0x000000010e1d235e in std::__1::function<void ()>::operator()() const at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/functional:1817
#12	0x000000010e1bf9ae in mbgl::util::AsyncTask::Impl::runTask() at /Users/mxn/hub/mapbox-gl-native/platform/darwin/src/async_task.cpp:45
#13	0x000000010e1bf865 in mbgl::util::AsyncTask::Impl::perform(void*) at /Users/mxn/hub/mapbox-gl-native/platform/darwin/src/async_task.cpp:50
#14	0x00000001020d9761 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#15	0x00000001020be907 in __CFRunLoopDoSources0 ()
#16	0x00000001020bde76 in __CFRunLoopRun ()
#17	0x00000001020bd884 in CFRunLoopRunSpecific ()
#18	0x000000010159c266 in -[XCTestExpectationWaiter primitiveWait:] ()
#19	0x000000010159bc47 in -[XCTestExpectationWaiter wait:forExpectations:enforceOrder:] ()
#20	0x00000001015bb859 in -[XCTestCase(AsynchronousTesting) waitForExpectationsWithTimeout:handler:] ()
#21	0x000000010df5993a in MGLDocumentationExampleTests.setUp() -> () at /Users/mxn/hub/mapbox-gl-native/platform/darwin/test/MGLDocumentationExampleTests.swift:37

Here’s the resource being loaded:

(lldb) p resource_.url
(std::__1::string) $0 = "mapbox://mapbox.mapbox-terrain-v2"

A little further up the stack, in mbgl::style::TileSourceImpl::loadDescription(), here’s the source that’s being loaded:

(lldb) p this->type
(const mbgl::SourceType) $1 = Vector
(lldb) p this->id
(std::__1::string) $2 = "my-source"

And further up, in mbgl::Map::Impl::Impl():

Printing description of this->this->styleURL:
(std::__1::string) styleURL = "mapbox://styles/mapbox/streets-v9"

my-source is a dummy source added in -[MGLStyleTests testAddingSourcesWithDuplicateIdentifiers], which is run before MGLDocumentationExampleTests. It’s unclear to me why one test would bleed into another like this.

@1ec5 1ec5 requested a review from friedbunny January 4, 2017 02:12
1ec5 added 13 commits January 4, 2017 10:55
Use test method names as names of example blocks and test method documentation comments as front matter for examples. Set off example blocks using a syntax similar to playground markup syntax. Avoid hard-coding indentation levels. Trigger Xcode build error when an error occurs in the script.
The comment said 200 while the code said 1,500.
Rewrote the example code insertion script to work on the original source files and overwrite any existing code examples on the same symbols. The script uses SourceKitten to find the documentation comment for the symbol named by the test method.

Replaced the Run Script build phase that runs the example code insertion script with a make rule that runs the same script. Inlined skeleton examples minus the contents of the code blocks.
1ec5 and others added 8 commits January 4, 2017 10:55
Refactored the example code insertion script to index test methods by their names, then recursively search the SourceKitten output for documentation comments that contain Swift code blocks, replacing each code block with the associated test method body.
The example code insertion script is now platform-agnostic.
Set the map view’s style to a minimal local JSON file. Wait for the style to finish loading before running each test. Corrected CGVector type.
The output of this mode isn’t a good indicator of whether any files would’ve needed to be changed, because the presence of a conditional compilation block in one of the test methods means this script would always change and revert the corresponding comment.
The iOS implementation of MGLMapView tries to show the Streets style by default even if no access token has been set. Avoid a race condition and frequent test failure by specifying the minimal style on initialization.
Keep map views from previous tests from hanging around, potentially obscuring the result of a subsequent test. Set the access token to a bogus token upfront for all style layer tests. Unified MGLStyle usage within MGLStyleTests.
@1ec5 1ec5 force-pushed the 1ec5-macos-docs-examples branch from 3a85e74 to 3e1a24f Compare January 4, 2017 18:56
@1ec5 1ec5 merged commit 50ae3eb into release-ios-v3.4.0 Jan 4, 2017
@1ec5 1ec5 deleted the 1ec5-macos-docs-examples branch January 4, 2017 20:19
@ericrwolfe ericrwolfe mentioned this pull request Mar 17, 2017
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants