add client setSubdomain and makeRequest tests. add https mock for testing. ...

add client setSubdomain and makeRequest tests.  add https mock for testing.  update test helper loading

CHANGELOG.md

@@ -2,6 +2,7 @@
 - Add tests (#7)
 - Fix crash on creating a client instance with no options
 - Fix challonge.createClient massageProperties is tautologically true (#2)
+- Fix malformed urls if an organization has a dash in the name (#15)
 
 ### 1.0
 - Initial Release

lib/api/client.js

@@ -93,8 +93,10 @@ Client.prototype.setSubdomain = function(subdomain) {
 	//generate the subdomain URL string if there is one
 	if (!subdomain) {
 		this.options.subdomain = '';
-	} else if (this.options.subdomain.indexOf('-') === -1) {
+	} else if (subdomain[subdomain.length-1] !== '-') {
 		this.options.subdomain = subdomain + '-';
+	} else {
+		this.options.subdomain = subdomain;
 	}
 };
 

lib/api/client.spec.js

-var Client = require('./client').Client;
+const qs = require('querystring');
+const Client = require('./client').Client;
+const httpsMock = require('https'); //replaced with mock by mockery
+
+function parseOpts() {
+	return qs.parse(httpsMock.opts.path.split('?')[1]);
+}
 
 describe('Client Class', () => {
 	describe('constructor', () => {
@@ -54,4 +60,140 @@ describe('Client Class', () => {
 			expect(client.options.get('someProp')).toBe('someVal');
 		});
 	});
+
+	describe('setSubdomain', () => {
+		let client;
+
+		beforeEach(() => {
+			client = new Client();
+		});
+
+		it('should set the subdomain to an empty string if it is not passed', () => {
+			[null, '', undefined, false].forEach(prop => {
+				client.setSubdomain(prop);
+				expect(client.options.subdomain).toBe('');
+			});
+		});
+
+		it('should set the subdomain to a passed value and append a -', () => {
+			client.setSubdomain('mydomain');
+			expect(client.options.subdomain).toBe('mydomain-');
+		});
+
+		it('should not append a - if the subdomain already has a dash', () => {
+			expect(client.options.subdomain).toBe('');
+			client.setSubdomain('mydomain-');
+			expect(client.options.subdomain).toBe('mydomain-');
+		});
+
+		it('should still append a - if the subdomain has a dash in the middle', () => {
+			expect(client.options.subdomain).toBe('');
+			client.setSubdomain('my-domain');
+			expect(client.options.subdomain).toBe('my-domain-');
+		});
+	});
+
+	describe('makeRequest', () => {
+		let client;
+
+		beforeEach(() => {
+			client = new Client();
+		});
+
+		it('should convert camelCased properties to underscores', () => {
+			client.makeRequest({
+				path: '/some/path',
+				method: 'GET',
+				someProperty: 'thing',
+				anotherProperty: 'anotherthing'
+			});
+
+			const opts = parseOpts();
+
+			expect(opts.some_property).toBe('thing');
+			expect(opts.another_property).toBe('anotherthing');
+		});
+
+		it('should recurse down properties to camelCase to underscore', () => {
+			client.makeRequest({
+				path: '/some/path',
+				method: 'GET',
+				someProperty: {
+					anotherProperty: 'anotherthing'
+				}
+			});
+
+			const opts = parseOpts();
+
+			expect(opts['some_property[another_property]']).toBe('anotherthing');
+		});
+
+		it('should add the api key to request', () => {
+			client.options.apiKey = 'mykey';
+			client.makeRequest({});
+
+			const opts = parseOpts();
+			expect(opts.api_key).toBe('mykey');
+		});
+
+		it('should add a random cache busting value', () => {
+			client.options.apiKey = 'mykey';
+			client.makeRequest({});
+
+			const opts = parseOpts();
+			expect(opts.cache_bust).toBeDefined();
+		});
+
+		it('should delete the callback, path, and method from the object', () => {
+			client.makeRequest({
+				callback: '',
+				path: '',
+				method: ''
+			});
+
+			const opts = parseOpts();
+			expect(opts.callback).not.toBeDefined();
+			expect(opts.path).not.toBeDefined();
+			expect(opts.method).not.toBeDefined();
+		});
+
+		it('should make an https request with the correct properties', () => {
+			spyOn(Math, 'random').and.returnValue(1);
+			client.options.apiKey = 'mykey';
+			client.makeRequest({
+				method: 'PUT'
+			});
+
+			expect(httpsMock.opts).toEqual({
+				hostname: 'api.challonge.com',
+				path: '/v1/tournaments.json?api_key=mykey&cache_bust=1',
+				method: 'PUT',
+				headers: {
+					'Content-Length': 0
+				}
+			});
+		});
+
+		it('should make an https request with the correct properties when there is complex data', () => {
+			spyOn(Math, 'random').and.returnValue(1);
+			client.options.apiKey = 'mykey';
+			client.makeRequest({
+				path: '/some/path',
+				method: 'GET',
+				randomprop: 'thingie',
+				someProperty: {
+					anotherProperty: 'anotherthing'
+				}
+			});
+
+			expect(httpsMock.opts).toEqual({
+				hostname: 'api.challonge.com',
+				path: '/v1/tournaments/some/path.json?randomprop=thingie&api_key=mykey&cache_bust=1&someProperty[another_property]=anotherthing&some_property[another_property]=anotherthing',
+				method: 'GET',
+				headers: {
+					'Content-Length': 0
+				}
+			});
+		});
+	});
 });

package.json

@@ -26,7 +26,8 @@
   "dependencies": {},
   "devDependencies": {
     "istanbul": "^0.4.5",
-    "jasmine": "^2.5.3"
+    "jasmine": "^2.5.3",
+    "mockery": "^2.0.0"
   },
   "scripts": {
     "test": "./node_modules/jasmine/bin/jasmine.js",

spec/helpers/index.js

+const mockery = require('mockery');
+mockery.enable({
+	warnOnReplace: false,
+    warnOnUnregistered: false
+});
+
+const httpsMock = {
+	opts: {},
+	listeners: {
+	},
+	request: function(opts, pipe) {
+		this.opts = opts;
+		pipe({
+			on: (method, cb) => {
+				if (!this.listeners[method]) { this.listeners[method] = []; }
+				this.listeners[method].push(cb);
+			}
+		});
+
+		return {
+			end: () => {}
+		};
+	}
+};
+
+mockery.registerMock('https', httpsMock);

spec/support/jasmine.json

@@ -4,7 +4,7 @@
     "**/*[sS]pec.js"
   ],
   "helpers": [
-    "spec/helpers/**/*.js"
+    "../spec/helpers/**/*.js"
   ],
   "stopSpecOnExpectationFailure": false,
   "random": false