Conversation
test/store_unittest.cc
Outdated
| protected: | ||
| MetadataStoreTest() : config(), store(config) {} | ||
|
|
||
| std::map<MonitoredResource, MetadataStore::Metadata> GetMetadataMap() const { |
There was a problem hiding this comment.
We got rid of this in #111 . GetMetadataMap() was made public.
| int server_threads, | ||
| const std::string& host, int port) | ||
| : MetadataApiServer(config, store, server_threads, host, port, | ||
| std::map<std::pair<std::string, std::string>, Handler>({ |
There was a problem hiding this comment.
You didn't have to make this change. First, you can test the Dispatcher class separately, and the HandleMonitoredResource callback separately (that's why the latter was pulled out in the first place). Second, the new code looks a lot uglier because of the explicit specs for the map and pair types. I would back out this whole commit.
There was a problem hiding this comment.
Looking things over now, I agree. Ill close this pull request and open a new one testing Dispatcher directly
There was a problem hiding this comment.
Hmm. I meant this commit, not this PR. You could have reused the PR for the dispatch tests (or the HandleMonitoredResource test, which you still can — just reopen the PR).
| EXPECT_EQ("", ""); | ||
| class ApiServerTest : public ::testing::Test { | ||
| protected: | ||
| static void CallDispatcher( |
There was a problem hiding this comment.
The MetadataApiServer::Dispatcher class is public (an oversight, probably — there's no need for it to be — but for now it is accessible), so you can just create instances in the test.
You would need to wrap HandleMonitoredResource into an accessor function like this, because that one is private.
3 participants
This commit 1) takes changes from dhrupadb-api-server-test and merges them against master as well as 2) adding tests for the dispatch operator in api_server