Skip to content

Commit

Permalink
Convert API service, last fm importer and tests to typescript (#786)
Browse files Browse the repository at this point in the history
* First cut at converting api-service.js to typescript

* Convert APIService.test.js to TypeScript

* Fix tests and add maximum stack depth

* Fix test files

* Fixed the tests in APIService.ts

* Fix tests in Importer.test.ts

* Tests for LastFMImporter fixed

* All tests pass

* Add package-lock.json and update entrypoint

* Port LastFm importer to typescript (#787)

* Port type.ts to type.d.ts

* Fix ESLint errors

* Fix LastFM importer icons (#788)

* Fix icons

* Disable no-unused-vars

* Add tests for skipping listens if non 429 error codes

Co-authored-by: Ishaan Shah <ishaanshah@users.noreply.github.com>
  • Loading branch information
paramsingh and ishaanshah committed Apr 11, 2020
1 parent 5b1a66e commit 0013ea3
Show file tree
Hide file tree
Showing 26 changed files with 783 additions and 675 deletions.
19 changes: 18 additions & 1 deletion .eslintrc.js
Expand Up @@ -26,15 +26,32 @@ module.exports = {
rules: {
"prettier/prettier": "error",
"react/jsx-filename-extension": "off",
"import/extensions": "off",
camelcase: "warn",
"no-unused-vars": "off",
"lines-between-class-members": [
"error",
"always",
{ exceptAfterSingleLine: true },
],
},
settings: {
"import/resolver": {
node: {
extensions: [".js", ".jsx", ".ts", ".tsx"],
},
},
},
overrides: [
{
files: ["**/*.test.js", "**/*.test.ts"],
files: ["**/*.test.js", "**/*.test.ts", "**/*.test.tsx"],
env: {
jest: true,
},
plugins: ["jest"],
rules: {
"import/no-extraneous-dependencies": "off",
"react/jsx-props-no-spreading": "off",
"jest/no-disabled-tests": "warn",
"jest/no-focused-tests": "error",
"jest/no-identical-title": "error",
Expand Down
2 changes: 1 addition & 1 deletion docker/Dockerfile.webpack
Expand Up @@ -3,5 +3,5 @@ FROM node:10.15-alpine
RUN mkdir /code
WORKDIR /code

COPY package.json package-lock.json webpack.config.js babel.config.js enzyme.config.js jest.config.js tsconfig.json .eslintrc.js .gitignore /code/
COPY package.json package-lock.json webpack.config.js babel.config.js enzyme.config.ts jest.config.js tsconfig.json .eslintrc.js .gitignore /code/
RUN npm install
5 changes: 0 additions & 5 deletions enzyme.config.js

This file was deleted.

5 changes: 5 additions & 0 deletions enzyme.config.ts
@@ -0,0 +1,5 @@
/* Used in jest.config.js */
import * as Enzyme from 'enzyme';
import * as Adapter from 'enzyme-adapter-react-16';

Enzyme.configure({ adapter: new Adapter() });
28 changes: 15 additions & 13 deletions jest.config.js
Expand Up @@ -2,36 +2,38 @@
// https://jestjs.io/docs/en/configuration.html

module.exports = {
preset: 'ts-jest',

// Automatically clear mock calls and instances between every test
clearMocks: true,

// An array of glob patterns indicating a set of files for which coverage information should be collected
collectCoverageFrom: ['<rootDir>/static/js/**/*.{ts,tsx,js,jsx}'],

// The directory where Jest should output its coverage files
coverageDirectory: 'coverage',

// An array of file extensions your modules use
moduleFileExtensions: ['ts', 'tsx', 'js', 'json', 'jsx'],

// The paths to modules that run some code to configure or set up the testing environment before each test
setupFiles: ['<rootDir>/enzyme.config.js'],
setupFiles: ['<rootDir>/enzyme.config.ts'],

// The test environment that will be used for testing
testEnvironment: 'jsdom',

// The glob patterns Jest uses to detect test files
testMatch: ['**/?(*.)+(spec|test).js?(x)'],
testMatch: ['**/?(*.)+(spec|test).(ts|js)?(x)'],

// An array of regexp pattern strings that are matched against all test paths, matched tests are skipped
testPathIgnorePatterns: ['\\\\node_modules\\\\'],

// This option sets the URL for the jsdom environment. It is reflected in properties such as location.href
testURL: 'http://localhost',

// An array of regexp pattern strings that are matched against all source file paths, matched files will skip transformation
transformIgnorePatterns: ['<rootDir>/node_modules/'],

// Indicates whether each individual test should be reported during the run
verbose: false,
verbose: true,
};
5 changes: 5 additions & 0 deletions listenbrainz/webserver/static/js/src/APIError.ts
@@ -0,0 +1,5 @@
export default class APIError extends Error {
status?: string;

response?: Response;
}
@@ -1,6 +1,4 @@
/* eslint-disable */
// TODO: Make the code ESLint compliant
import APIService from "./api-service";
import APIService from "./APIService";

const apiService = new APIService("foobar");

Expand All @@ -14,21 +12,7 @@ describe("submitListens", () => {
});
});

// Mock function for setTimeout and console.warn
console.warn = jest.fn();
window.setTimeout = jest.fn();
});

it("throws an error if userToken is not a string", async () => {
await expect(
apiService.submitListens(["foo", "bar"], "import", "foobar")
).rejects.toThrow(SyntaxError);
});

it("throws an error if listenType is invalid", async () => {
await expect(
apiService.submitListens("foobar", "foobar", "foobar")
).rejects.toThrow(SyntaxError);
jest.useFakeTimers();
});

it("calls fetch with correct parameters", async () => {
Expand All @@ -48,13 +32,20 @@ describe("submitListens", () => {

it("retries if submit fails", async () => {
// Overide mock for fetch
window.fetch = jest.fn().mockImplementation(() => {
return Promise.reject();
});
window.fetch = jest
.fn()
.mockImplementationOnce(() => {
return Promise.reject(Error);
})
.mockImplementation(() => {
return Promise.resolve({
ok: true,
status: 200,
});
});

await apiService.submitListens("foobar", "import", "foobar");
expect(console.warn).toHaveBeenCalledWith("Error, retrying in 3 sec");
expect(window.setTimeout).toHaveBeenCalledTimes(1);
expect(setTimeout).toHaveBeenCalledTimes(1);
});

it("retries if error 429 is recieved fails", async () => {
Expand All @@ -67,8 +58,7 @@ describe("submitListens", () => {
});

await apiService.submitListens("foobar", "import", "foobar");
expect(console.warn).toHaveBeenCalledWith("Error, retrying in 3 sec");
expect(window.setTimeout).toHaveBeenCalledTimes(1);
expect(setTimeout).toHaveBeenCalledTimes(1);
});

it("skips if any other response code is recieved", async () => {
Expand All @@ -81,7 +71,7 @@ describe("submitListens", () => {
});

await apiService.submitListens("foobar", "import", "foobar");
expect(console.warn).toHaveBeenCalledWith("Got 404 error, skipping");
expect(setTimeout).not.toHaveBeenCalled(); // no setTimeout calls for future retries
});

it("returns the response if successful", async () => {
Expand Down Expand Up @@ -121,12 +111,6 @@ describe("getLatestImport", () => {
apiService.checkStatus = jest.fn();
});

it("throws an error if userName is not a string", async () => {
await expect(apiService.getLatestImport(["foo", "bar"])).rejects.toThrow(
SyntaxError
);
});

it("encodes url correctly", async () => {
await apiService.getLatestImport("ईशान");
expect(window.fetch).toHaveBeenCalledWith(
Expand Down Expand Up @@ -161,18 +145,6 @@ describe("setLatestImport", () => {
apiService.checkStatus = jest.fn();
});

it("throws an error if userToken is not a string", async () => {
await expect(apiService.setLatestImport(["foo", "bar"], 0)).rejects.toThrow(
SyntaxError
);
});

it("throws an error if timestamp is not a number", async () => {
await expect(apiService.setLatestImport("foobar", "0")).rejects.toThrow(
SyntaxError
);
});

it("calls fetch with correct parameters", async () => {
await apiService.setLatestImport("foobar", 0);
expect(window.fetch).toHaveBeenCalledWith("foobar/1/latest-import", {
Expand Down

0 comments on commit 0013ea3

Please sign in to comment.