Add HttpPitMinter IT #639
Add HttpPitMinter IT #639
Conversation
| assertEquals(pid, "baz"); | ||
| } | ||
|
|
||
| private String startServer(final String data) throws IOException { |
There was a problem hiding this comment.
Is there a reason you are not using the standard Grizzly startup pattern? I am a little concerned about port conflicts when running this with other services, i.e. not using build-helper-maven-plugin
There was a problem hiding this comment.
Well, the test is starting Grizzly with PortRange, which does the same job of port reservation and assignment as builder-helper-maven-plugin (or so it seems to me). I could definitely increase the number of ports to try, though.
There was a problem hiding this comment.
For consistency with the rest of the Fedora codebase, please use the build-helper-maven-plugin for test port management.
| */ | ||
| @RunWith(SpringJUnit4ClassRunner.class) | ||
| @ContextConfiguration("/spring-test/test-container.xml") | ||
| public class HttpPidMinterIT { |
There was a problem hiding this comment.
This test class should be in fcrepo-kernel-impl, not fcrepo-http-api.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.glassfish.jersey.core</groupId> | ||
| <artifactId>jersey-server</artifactId> |
There was a problem hiding this comment.
Instead of adding these dependencies to -kernel-impl, why not yank the HttpPidMinter out into a separate module? I'm a little worried doing this will a) give people the wrong idea about -kernel, and b) make it harder to detect accidental inclusion of HTTP-specific behavior that this level (which really, really shouldn't care about HTTP)
There was a problem hiding this comment.
I can't believe I didn't notice this! What on Earth is a class named HttpPidMinter doing in -kernel-impl? +1 to what @cbeer says and -1 to @fedoradmin for putting this class in here.
|
Resolved with: cd2d11c |
https://www.pivotaltracker.com/story/show/81483926