If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Status

Webmaker
MakeAPI
RESOLVED WONTFIX
4 years ago
10 months ago

People

(Reporter: humph, Unassigned)

Tracking

Details

(Whiteboard: s=2013w29 p=1)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
We need tests for the MakeAPI, similar to how we are doing it for login.  I want things tested per commit so we don't keep regressing.  I'll start this and get sedge to finish it.
(Reporter)

Comment 1

4 years ago
WIP here https://github.com/mozilla/MakeAPI/pull/67
(Reporter)

Comment 2

4 years ago
Sedge, I've gotten this to the point that I can hand it off to you to complete, see https://github.com/mozilla/MakeAPI/pull/67.  Similar to the Login tests, my goal is maximal coverage of the code paths through the MakeAPI routes.  Ideally, whenever we make changes, or check-in code, we'll know that we haven't broken anything in the API.

I've CC'ed Chris and Matt, as they know more about the MakeAPI than I do, and can guide you as you write these tests.  They are also probably the right people to do reviews, especially if I'm away when you get it done.

A few notes about my code:

* When the tests start they spin up a bunch of servers: the MakeAPI on port 3000; Webfaker on ports 3001, 3002, 3003.  We only use the fake Login server on 3002 to simulate having a real login server available to us.  You may find that the Webfaker stuff isn't sufficient for what you need, and we either need to a) update it; b) run a real login.webmaker.org server in the test environment.  Either is fine.

* In Webfaker I've made 2 accounts, one called admin and one called notadmin.  They can be used when you need to create records as an admin or non-admin

* Similar to how the login tests worked, I've created a unique() function which builds a unique make record.  It takes an arg where you can pass options with specific data, if you need to specify how things get set.  The code will show you how it works.

Let me know if you have questions.
Haven't forgotten about this bad-boy, but duty has called. Repeatedly.
Created attachment 766904 [details] [review]
https://github.com/igoryen/MakeAPI/pull/2
Attachment #766904 - Flags: review?(kieran.sedgwick)
Comment on attachment 766904 [details] [review]
https://github.com/igoryen/MakeAPI/pull/2

Keep hacking! 

Process point: It is a good idea to mark pull requests that you /don't/ want to be merged.  I do this by prefixing the PR name with [WIP] (work in progress).

Flag again when you have more for me to look at.
Attachment #766904 - Flags: review?(kieran.sedgwick) → review-
Comment on attachment 766904 [details] [review]
https://github.com/igoryen/MakeAPI/pull/2

https://github.com/igoryen/MakeAPI/pull/1/files
Attachment #766904 - Flags: review- → review?(kieran.sedgwick)
Comment on attachment 766904 [details] [review]
https://github.com/igoryen/MakeAPI/pull/2

Done! Take a look at my comments.
Attachment #766904 - Flags: review?(kieran.sedgwick) → review-
Comment on attachment 766904 [details] [review]
https://github.com/igoryen/MakeAPI/pull/2

https://github.com/igoryen/MakeAPI/pull/2
Attachment #766904 - Flags: review- → review?(kieran.sedgwick)
Comment on attachment 766904 [details] [review]
https://github.com/igoryen/MakeAPI/pull/2

https://github.com/igoryen/MakeAPI/pull/2
Attachment #766904 - Attachment description: https://github.com/igoryen/MakeAPI/pull/1 → https://github.com/igoryen/MakeAPI/pull/2
Comment on attachment 766904 [details] [review]
https://github.com/igoryen/MakeAPI/pull/2

You're getting there!  Keep posting these patches.
Attachment #766904 - Flags: review?(kieran.sedgwick) → review-
Whiteboard: c=makeapi s=2013w29

Updated

4 years ago
Whiteboard: c=makeapi s=2013w29 → s=2013w29 p=1
(Assignee)

Updated

4 years ago
Attachment mime type: text/plain → text/x-github-pull-request
It's been so long - has this been done already?
Flags: needinfo?(cade)
The only tests that exist are for the DSL generation code.
Flags: needinfo?(cade)
I really wanna tackle this bug, but I have no idea when I'll get to it since I'm back in school. Would you be comfortable leaving it on me until I can do it? Otherwise I'll un-assign it.
Flags: needinfo?(cade)
Feel free to leave this assigned to yourself. If I find myself with the time to take this on, I'll know where to look.
Flags: needinfo?(cade)
Assignee: kieran.sedgwick → nobody
Status: ASSIGNED → NEW

Updated

10 months ago
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.