{"action":"opened","issue":{"url":"https://api.github.com/repos/studentinsights/studentinsights/issues/1327","repository_url":"https://api.github.com/repos/studentinsights/studentinsights","labels_url":"https://api.github.com/repos/studentinsights/studentinsights/issues/1327/labels{/name}","comments_url":"https://api.github.com/repos/studentinsights/studentinsights/issues/1327/comments","events_url":"https://api.github.com/repos/studentinsights/studentinsights/issues/1327/events","html_url":"https://github.com/studentinsights/studentinsights/issues/1327","id":283016492,"number":1327,"title":"Best way to test Api.js?","user":{"login":"alexsoble","id":3209501,"avatar_url":"https://avatars3.githubusercontent.com/u/3209501?v=4","gravatar_id":"","url":"https://api.github.com/users/alexsoble","html_url":"https://github.com/alexsoble","followers_url":"https://api.github.com/users/alexsoble/followers","following_url":"https://api.github.com/users/alexsoble/following{/other_user}","gists_url":"https://api.github.com/users/alexsoble/gists{/gist_id}","starred_url":"https://api.github.com/users/alexsoble/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexsoble/subscriptions","organizations_url":"https://api.github.com/users/alexsoble/orgs","repos_url":"https://api.github.com/users/alexsoble/repos","events_url":"https://api.github.com/users/alexsoble/events{/privacy}","received_events_url":"https://api.github.com/users/alexsoble/received_events","type":"User","site_admin":false},"labels":[{"id":323281979,"url":"https://api.github.com/repos/studentinsights/studentinsights/labels/question","name":"question","color":"cc317c","default":true}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-12-18T20:52:56Z","updated_at":"2017-12-18T20:52:56Z","closed_at":null,"author_association":"OWNER","body":"I added the first test file for Api.js in #1326. Right now it's very minimal, it just tests whether the class decides correctly between create note/update note. \r\n\r\nI'd like to add more thorough tests, but I ran into a couple of questions that I'd love to talk about with other folks who are interested in JS testing choices! \r\n\r\n**Injecting dependencies.** I started experimenting with Jest mocks, but quickly found them hard to inject into the code. How do folks feel about refactoring so we can inject our HTTP-request-sending code as dependencies? \r\n\r\nThis would be something like:\r\n\r\n```\r\n// current\r\n\r\n  _updateNote(studentId, eventNoteParams) {\r\n    return this._put('/students/' + studentId + '/event_notes/' + id + '.json', {\r\n      event_note: {\r\n        ...\r\n        text: eventNoteParams.text,\r\n      }\r\n    });\r\n  }\r\n\r\n// refactor\r\n\r\n  _updateNote(studentId, eventNoteParams, putFn) {\r\n    return putFn('/students/' + studentId + '/event_notes/' + id + '.json', {\r\n      event_note: {\r\n        ...\r\n        text: eventNoteParams.text,\r\n      }\r\n    });\r\n  }\r\n```\r\n\r\nThat way we could pass in `this._put` in the feature side code, and pass in a Jest mock in the test code. More flexible! What do folks think? \r\n\r\n**Private functions.** Some of the meatiest logic in Api.js is in `_createNote` and `_updateNote`. I know the leading underscore denotes a private function, and I know private functions generally aren't tested. So I'm not sure what to make of that! \r\n\r\nAt first I thought that these functions had a leading underscore because they use jQuery instead of fetch as their API, and that was a \"likely to change\" choice that we wanted to hide away as an implementation detail. But `_createNote` and `_updateNote` don't call either jQuery or fetch, instead they pass data on to `_post` and `_put`. \r\n\r\ntl;dr I want to add tests for `_createNote` and `_updateNote` but I'm pausing because they're private functions, and maybe there's something I'm missing about the intent of the code! @kevinrobinson I believe you wrote these so let me know if you have anything to add. Clearly not a high-priority topic, just programming discussion. "}}
{"actor":{"display_login":"alexsoble"}}