Github user mitchell852 commented on a diff in the pull request:

    
https://github.com/apache/incubator-trafficcontrol/pull/403#discussion_r112017704
  
    --- Diff: traffic_ops/app/t/api/1.2/deliveryservice.t ---
    @@ -30,302 +30,407 @@ use Test::TestHelper;
     
     BEGIN { $ENV{MOJO_MODE} = "test" }
     
    -my $schema = Schema->connect_to_database;
    -my $dbh    = Schema->database_handle;
    -my $t      = Test::Mojo->new('TrafficOps');
     
    -Test::TestHelper->unload_core_data($schema);
    -Test::TestHelper->load_core_data($schema);
    -
    -ok $t->post_ok( '/login', => form => { u => Test::TestHelper::ADMIN_USER, 
p => Test::TestHelper::ADMIN_USER_PASSWORD } )->status_is(302)
    -   ->or( sub { diag $t->tx->res->content->asset->{content}; } ), 'Should 
login?';
    -
    -# It gets existing delivery services
    -ok $t->get_ok("/api/1.2/deliveryservices/list")->status_is(200)->or( sub { 
diag $t->tx->res->content->asset->{content} } )
    -           ->json_is( "/response/0/xmlId", "steering-ds1" )
    -           ->json_is( "/response/0/logsEnabled", 0 )
    -           ->json_is( "/response/0/ipv6RoutingEnabled", 1 )
    -           ->json_is( "/response/1/xmlId", "steering-ds2" );
    -
    -ok 
$t->get_ok("/api/1.2/deliveryservices/list?logsEnabled=true")->status_is(200)->or(
 sub { diag $t->tx->res->content->asset->{content} } )
    -           ->json_is( "/response/0/xmlId", "test-ds1" )
    -           ->json_is( "/response/0/logsEnabled", 1 )
    -           ->json_is( "/response/0/ipv6RoutingEnabled", 1 )
    -           ->json_is( "/response/1/xmlId", "test-ds4" );
    -
    -# It creates new delivery services
    -ok $t->post_ok('/api/1.2/deliveryservices/create' => {Accept => 
'application/json'} => json => {
    -        "xmlId" => "ds_1",
    -        "displayName" => "ds_displayname_1",
    -        "protocol" => "1",
    -        "orgServerFqdn" => "http://10.75.168.91";,
    -        "cdnName" => "cdn1",
    -        "profileName" => "CCR1",
    -        "type" => "HTTP",
    -        "multiSiteOrigin" => "0",
    -        "regionalGeoBlocking" => "1",
    -        "active" => "false",
    -        "matchList" => [
    -            {
    -                "type" =>  "HOST_REGEXP",
    -                "setNumber" =>  "0",
    -                "pattern" => ".*\\.ds_1\\..*"
    -            },
    -            {
    -                "type" =>  "HOST_REGEXP",
    -                "setNumber" =>  "1",
    -                "pattern" => ".*\\.my_vod1\\..*"
    -            }
    -        ]})
    -    ->status_is(200)->or( sub { diag 
$t->tx->res->content->asset->{content}; } )
    -    ->json_is( "/response/xmlId" => "ds_1")
    -    ->json_is( "/response/displayName" => "ds_displayname_1")
    -    ->json_is( "/response/orgServerFqdn" => "http://10.75.168.91";)
    -    ->json_is( "/response/cdnName" => "cdn1")
    -    ->json_is( "/response/profileName" => "CCR1")
    -    ->json_is( "/response/protocol" => "1")
    -    ->json_is( "/response/type" => "HTTP")
    -    ->json_is( "/response/multiSiteOrigin" => "0")
    -    ->json_is( "/response/regionalGeoBlocking" => "1")
    -    ->json_is( "/response/active" => "false")
    -    ->json_is( "/response/matchList/0/type" => "HOST_REGEXP")
    -    ->json_is( "/response/matchList/0/setNumber" => "0")
    -    ->json_is( "/response/matchList/0/pattern" => ".*\\.ds_1\\..*")
    -    ->json_is( "/response/matchList/1/type" => "HOST_REGEXP")
    -    ->json_is( "/response/matchList/1/setNumber" => "1")
    -    ->json_is( "/response/matchList/1/pattern" => ".*\\.my_vod1\\..*")
    -            , 'Does the deliveryservice details return?';
    -
    -ok $t->post_ok('/api/1.2/deliveryservices/create' => {Accept => 
'application/json'} => json => {
    -        "xmlId" => "ds_2",
    -        "displayName" => "ds_displayname_2",
    -        "protocol" => "1",
    -        "orgServerFqdn" => "http://10.75.168.91";,
    -        "cdnName" => "cdn1",
    -        "profileName" => "CCR1",
    -        "type" => "HTTP",
    -        "multiSiteOrigin" => "0",
    -        "regionalGeoBlocking" => "1",
    -        "active" => "false",
    -        "matchList" => [
    -            {
    -                "type" =>  "HOST_REGEXP",
    -                "setNumber" =>  "0",
    -                "pattern" => ".*\\.ds_2\\..*"
    -            },
    -            {
    -                "type" =>  "HOST_REGEXP",
    -                "setNumber" =>  "1",
    -                "pattern" => ".*\\.my_vod2\\..*"
    -            }
    -        ]})
    -    ->status_is(200)->or( sub { diag 
$t->tx->res->content->asset->{content}; } )
    -    ->json_is( "/response/xmlId" => "ds_2")
    -    ->json_is( "/response/displayName" => "ds_displayname_2")
    -    ->json_is( "/response/orgServerFqdn" => "http://10.75.168.91";)
    -    ->json_is( "/response/cdnName" => "cdn1")
    -    ->json_is( "/response/profileName" => "CCR1")
    -    ->json_is( "/response/protocol" => "1")
    -    ->json_is( "/response/type" => "HTTP")
    -    ->json_is( "/response/multiSiteOrigin" => "0")
    -    ->json_is( "/response/regionalGeoBlocking" => "1")
    -    ->json_is( "/response/active" => "false")
    -    ->json_is( "/response/matchList/0/type" => "HOST_REGEXP")
    -    ->json_is( "/response/matchList/0/setNumber" => "0")
    -    ->json_is( "/response/matchList/0/pattern" => ".*\\.ds_2\\..*")
    -    ->json_is( "/response/matchList/1/type" => "HOST_REGEXP")
    -    ->json_is( "/response/matchList/1/setNumber" => "1")
    -    ->json_is( "/response/matchList/1/pattern" => ".*\\.my_vod2\\..*")
    -            , 'Does the deliveryservice details return?';
    -
    -# It allows adding a delivery service to a different CDN with a matching 
regex in another CDN's delivery service
    -ok $t->post_ok('/api/1.2/deliveryservices/create' => {Accept => 
'application/json'} => json => {
    -        "xmlId" => "ds_3",
    -        "displayName" => "ds_displayname_1",
    -        "protocol" => "1",
    -        "orgServerFqdn" => "http://10.75.168.91";,
    -        "cdnName" => "cdn2",
    -        "profileName" => "CCR2",
    -        "type" => "HTTP",
    -        "multiSiteOrigin" => "0",
    -        "regionalGeoBlocking" => "1",
    -        "active" => "false",
    -        "matchList" => [
    -            {
    -                "type" =>  "HOST_REGEXP",
    -                "setNumber" =>  "0",
    -                "pattern" => ".*\\.ds_1\\..*"
    -            },
    -            {
    -                "type" =>  "HOST_REGEXP",
    -                "setNumber" =>  "1",
    -                "pattern" => ".*\\.my_vod2\\..*"
    -            }
    -        ]})
    -    ->status_is(200)->or( sub { diag 
$t->tx->res->content->asset->{content}; } )
    -    ->json_is( "/response/xmlId" => "ds_3")
    -    ->json_is( "/response/displayName" => "ds_displayname_1")
    -    ->json_is( "/response/orgServerFqdn" => "http://10.75.168.91";)
    -    ->json_is( "/response/cdnName" => "cdn2")
    -    ->json_is( "/response/profileName" => "CCR2")
    -    ->json_is( "/response/protocol" => "1")
    -    ->json_is( "/response/type" => "HTTP")
    -    ->json_is( "/response/multiSiteOrigin" => "0")
    -    ->json_is( "/response/regionalGeoBlocking" => "1")
    -    ->json_is( "/response/active" => "false")
    -    ->json_is( "/response/matchList/0/type" => "HOST_REGEXP")
    -    ->json_is( "/response/matchList/0/setNumber" => "0")
    -    ->json_is( "/response/matchList/0/pattern" => ".*\\.ds_1\\..*")
    -    ->json_is( "/response/matchList/1/type" => "HOST_REGEXP")
    -    ->json_is( "/response/matchList/1/setNumber" => "1")
    -    ->json_is( "/response/matchList/1/pattern" => ".*\\.my_vod2\\..*")
    -            , 'Does the deliveryservice details return?';
    -
    -my $ds_id = &get_ds_id('ds_1');
    -
    -ok $t->put_ok('/api/1.2/deliveryservices/' . $ds_id . '/update'  => 
{Accept => 'application/json'} => json => {
    -        "xmlId" => "ds_1",
    -        "displayName" => "ds_displayname_11",
    -        "protocol" => "2",
    -        "orgServerFqdn" => "http://10.75.168.91";,
    -        "cdnName" => "cdn1",
    -        "profileName" => "CCR1",
    -        "type" => "HTTP",
    -        "multiSiteOrigin" => "0",
    -        "regionalGeoBlocking" => "0",
    -        "active" => "true",
    -        "matchList" => [
    -            {
    -                "type" =>  "HOST_REGEXP",
    -                "setNumber" =>  "0",
    -                "pattern" => ".*\\.my_vod1\\..*"
    -            }
    -        ]})
    -    ->status_is(200)->or( sub { diag 
$t->tx->res->content->asset->{content}; } )
    -    ->json_is( "/response/xmlId" => "ds_1")
    -    ->json_is( "/response/displayName" => "ds_displayname_11")
    -    ->json_is( "/response/orgServerFqdn" => "http://10.75.168.91";)
    -    ->json_is( "/response/cdnName" => "cdn1")
    -    ->json_is( "/response/profileName" => "CCR1")
    -    ->json_is( "/response/protocol" => "2")
    -    ->json_is( "/response/type" => "HTTP")
    -    ->json_is( "/response/multiSiteOrigin" => "0")
    -    ->json_is( "/response/regionalGeoBlocking" => "0")
    -    ->json_is( "/response/active" => "true")
    -    ->json_is( "/response/matchList/0/type" => "HOST_REGEXP")
    -    ->json_is( "/response/matchList/0/setNumber" => "0")
    -    ->json_is( "/response/matchList/0/pattern" => ".*\\.my_vod1\\..*")
    -            , 'Does the deliveryservice details return?';
    -
    -
    -# It allows updating a delivery service in a CDN with a matching regex in 
another CDN's delivery service
    -
    -$ds_id = &get_ds_id('ds_3');
    -
    -ok $t->put_ok('/api/1.2/deliveryservices/' . $ds_id . '/update' => {Accept 
=> 'application/json'} => json => {
    -           "xmlId" => "ds_3",
    -           "displayName" => "ds_displayname_1",
    -           "protocol" => "1",
    -           "orgServerFqdn" => "http://10.75.168.91";,
    -           "cdnName" => "cdn2",
    -           "profileName" => "CCR2",
    -           "type" => "HTTP",
    -           "multiSiteOrigin" => "0",
    -           "regionalGeoBlocking" => "1",
    -           "active" => "false",
    -           "matchList" => [
    -                   {
    -                           "type" =>  "HOST_REGEXP",
    -                           "setNumber" =>  "0",
    -                           "pattern" => ".*\\.ds_1\\..*"
    -                   },
    -                   {
    -                           "type" =>  "HOST_REGEXP",
    -                           "setNumber" =>  "1",
    -                           "pattern" => ".*\\.my_vod1\\..*"
    -                   }
    -           ]})
    -   ->status_is(200)->or( sub { diag 
$t->tx->res->content->asset->{content}; } )
    -           ->status_is(200)->or( sub { diag 
$t->tx->res->content->asset->{content}; } )
    -           ->json_is( "/response/xmlId" => "ds_3")
    -           ->json_is( "/response/displayName" => "ds_displayname_1")
    -           ->json_is( "/response/orgServerFqdn" => "http://10.75.168.91";)
    -           ->json_is( "/response/cdnName" => "cdn2")
    -           ->json_is( "/response/profileName" => "CCR2")
    -           ->json_is( "/response/protocol" => "1")
    -           ->json_is( "/response/type" => "HTTP")
    -           ->json_is( "/response/multiSiteOrigin" => "0")
    -           ->json_is( "/response/regionalGeoBlocking" => "1")
    -           ->json_is( "/response/active" => "false")
    -           ->json_is( "/response/matchList/0/type" => "HOST_REGEXP")
    -           ->json_is( "/response/matchList/0/setNumber" => "0")
    -           ->json_is( "/response/matchList/0/pattern" => ".*\\.ds_1\\..*")
    -           ->json_is( "/response/matchList/1/type" => "HOST_REGEXP")
    -           ->json_is( "/response/matchList/1/setNumber" => "1")
    -           ->json_is( "/response/matchList/1/pattern" => 
".*\\.my_vod1\\..*")
    -   , 'Does the deliveryservice details return?';
    -
    -# It does not allow changing a regex to be the same as an existing one in 
another DS in the same CDN
    -$ds_id = &get_ds_id('ds_1');
    -ok $t->put_ok('/api/1.2/deliveryservices/' . $ds_id . '/update'  => 
{Accept => 'application/json'} => json => {
    -                   "xmlId" => "ds_1",
    -                   "displayName" => "ds_displayname_2",
    +sub run_ut {
    --- End diff --
    
    Is there a reason you put everything inside run_ut(). I don't think it is 
done this way in other places. And by doing this, I can't really tell what 
you've changed in this file as it looks like you changed everything.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to