rob05c commented on a change in pull request #2191: revert changes that broke 
perl unit tests
URL: 
https://github.com/apache/incubator-trafficcontrol/pull/2191#discussion_r185235867
 
 

 ##########
 File path: traffic_ops/app/lib/TrafficOpsRoutes.pm
 ##########
 @@ -449,6 +449,13 @@ sub api_routes {
        # -- CDNS: ROUTING
        $r->get("/api/$version/cdns/routing")->over( authenticated => 1, 
not_ldap => 1 )->to( 'Cdn#routing', namespace => $namespace );
 
+       # -- CDNS: SNAPSHOT
+       $r->get("/api/$version/cdns/:name/snapshot")->over( authenticated => 1, 
not_ldap => 1 )->to( 'Topology#get_snapshot', namespace => $namespace );
+       $r->get("/api/$version/cdns/:name/snapshot/new")->over( authenticated 
=> 1, not_ldap => 1 )->to( 'Topology#get_new_snapshot', namespace => $namespace 
);
+       $r->put( "/api/$version/cdns/:id/snapshot" => [ id => qr/\d+/ ] 
)->over( authenticated => 1, not_ldap => 1 )
+       ->to( 'Topology#SnapshotCRConfig', namespace => $namespace );
+       $r->put("/api/$version/snapshot/:cdn_name")->over( authenticated => 1, 
not_ldap => 1 )->to( 'Topology#SnapshotCRConfig', namespace => $namespace );
 
 Review comment:
   It looks like all those tests are doing is, after doing their own 
cdn/ds/config stuff, is getting or setting the snapshot.
   
   The Go CRConfig doesn't have API tests, only unit tests, because API Test 
DSes and Servers didn't exist when I wrote it; but they do now.
   
   For the Perl tests, for cdns and deliveryservices, all they're doing is 
trying to snapshot and get the snapshot, after posting their own data. Which, 
the CRConfig API Tests will already do, creating DSes and CDNS and other things 
(and in fact, actually verify the things it created are in the CRConfig, which 
is more than the Perl tests do).
   
   For the config tests, all they're doing is GETs of config files, no POSTs, 
and then getting the CRConfig. So, they're not actually changing data, so 
there's really no reason to verify the CRConfig GET succeeds there, we'll 
already be verifying that in the CRConfig's own tests.
   
   So, I'd vote we write API Tests for the CRConfig, and once those exist, 
remove the CRConfig calls in all the Perl tests.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to