Arrow (with Mocha + Chai) tests for node.js example code #26

Merged
merged 14 commits into from Jan 8, 2014

Conversation

Projects
None yet
2 participants
Member

maxiaohao commented Dec 19, 2013

No description provided.

@nottoseethesun nottoseethesun commented on an outdated diff Dec 23, 2013

example/nodejs/RunExamples.js
+var args = process.argv;
+
+if (args) {
+
+ var AWS, ec2;
+ AWS = require('aws-sdk');
+ AWS.config.update({
+ accessKeyId: 'foo',
+ secretAccessKey: 'bar',
+ region: 'baz'
+ });
+ ec2 = new AWS.EC2({
+ endpoint: new AWS.Endpoint('http://localhost:9090/')
+ });
+
+ var action = args[2];
@nottoseethesun

nottoseethesun Dec 23, 2013

Owner

Consolidate all 'var's at the top of the module. If there are too many, then break up the module into two or more modules.

@nottoseethesun nottoseethesun commented on an outdated diff Dec 23, 2013

example/nodejs/RunExamples.js
+ AWS = require('aws-sdk');
+ AWS.config.update({
+ accessKeyId: 'foo',
+ secretAccessKey: 'bar',
+ region: 'baz'
+ });
+ ec2 = new AWS.EC2({
+ endpoint: new AWS.Endpoint('http://localhost:9090/')
+ });
+
+ var action = args[2];
+ var instanceIDs = args.slice(3, args.length);
+
+ switch (action) {
+ case 'runInstances':
+ var runInstancesExample = require('./simple/RunInstancesExample.js');
@nottoseethesun

nottoseethesun Dec 23, 2013

Owner

Make all these "bodies" to the 'switch' statement into their own functions. That way, your 'switch' statement stays very short and readers can see exactly what the logic of it is, at a single glance. Move the functions down below, as they then represent bundled, forgettable detail.

@nottoseethesun nottoseethesun commented on an outdated diff Dec 23, 2013

example/nodejs/full/DescribeImagesExample.js
@@ -1,28 +1,17 @@
'use strict';
-var AWS, ec2;
+exports.describeImages = function(ec2, fnCallback) {
+ // describe all AMIs by passing no filter params
+ ec2.describeImages({},
+ function handleResponse(err, resp) {
@nottoseethesun

nottoseethesun Dec 23, 2013

Owner

Needs a more descriptive name.

Owner

nottoseethesun commented Dec 26, 2013

As long as the Arrow timeout issue with Mocha is not blocking this, :shipit:

@nottoseethesun nottoseethesun commented on an outdated diff Dec 26, 2013

@@ -175,7 +176,7 @@ uploadArchives {
developer {
id 'Jayscrivner'
name 'Jay Scrivner'
- email 'Jayscrivner@gmail.com'
+ email 'Jayscrivner@gmail.com'
@nottoseethesun

nottoseethesun Dec 26, 2013

Owner

We shouldn't have any real e-mail addresses in public code. Pls fix.

@nottoseethesun nottoseethesun commented on an outdated diff Dec 26, 2013

example/nodejs/test/TestExamples.js
+ it('should start ' + runCount + ' new pending instances successfully', function(done) {
+ runInstancesExample.runInstances(exampleImageID, instanceType, runCount, ec2, function getInstances(instances) {
+ expect(instances).to.have.length(runCount);
+ instances.forEach(function(inst) {
+ expect(inst.State.Name).to.equal('pending');
+ exampleInstanceIDs.push(inst.InstanceId);
+ });
+ // pick one of the instanceID for the subsequent tests (stop/start/terminate)
+ exampleInstanceID = exampleInstanceIDs[exampleInstanceIDs.length - 1];
+ done();
+ });
+ });
+
+ it('should find all the ' + runCount + ' instances turned into running after ' + maxBootSeconds * 2 + ' seconds', function(done) {
+ // alter the timeout to maxBootSeconds*2+1 seconds to make sure the test can wait until instances turned into running
+ this.timeout((maxBootSeconds * 2 + 1) * 1000);
@nottoseethesun

nottoseethesun Dec 26, 2013

Owner

Move '2' and '1' here out to named pseudo-constants. Also see my comment below.

@nottoseethesun nottoseethesun commented on the diff Dec 26, 2013

example/nodejs/test/TestExamples.js
+ describeInstancesExample.describeInstances(exampleInstanceIDs, ec2, function getInstances(instances) {
+ expect(instances).to.have.length(runCount);
+ instances.forEach(function(inst) {
+ expect(inst.State.Name).to.equal('running');
+ });
+ done();
+ });
+ },
+ 1000 * 2 * maxBootSeconds);
+ });
+ });
+
+ describe('Stop Instance Test -> ', function() {
+ it('should stop one picked instance successfully', function(done) {
+ stopInstancesExample.stopInstances([exampleInstanceID], ec2, function getInstances(instances) {
+ expect(instances).to.have.length(1);
@nottoseethesun

nottoseethesun Dec 26, 2013

Owner

'1' here is okay as it is, since it is a simple length value test.

@nottoseethesun nottoseethesun commented on an outdated diff Dec 26, 2013

example/nodejs/test/TestExamples.js
+
+ describe('Stop Instance Test -> ', function() {
+ it('should stop one picked instance successfully', function(done) {
+ stopInstancesExample.stopInstances([exampleInstanceID], ec2, function getInstances(instances) {
+ expect(instances).to.have.length(1);
+ instances.forEach(function(inst) {
+ expect(inst.PreviousState.Name).to.equal('running');
+ expect(inst.CurrentState.Name).to.equal('stopping');
+ });
+ done();
+ });
+ });
+
+ it('should find that instance turned into stopped after ' + maxShutdownSeconds * 2 + ' seconds', function(done) {
+ // alter the timeout to maxShutdownSeconds*2+1 seconds to make sure the test can wait until instances turned into stopped
+ this.timeout((maxShutdownSeconds * 2 + 1) * 1000);
@nottoseethesun

nottoseethesun Dec 26, 2013

Owner

Move this repeated calculation out to a helper function.

maxiaohao merged commit 14e25fb into master Jan 8, 2014

1 check was pending

default The Travis CI build is in progress
Details

maxiaohao deleted the arrow-tests branch Jan 9, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment