startersacademy / fullstack-project-01

Learning Management System
MIT License
0 stars 5 forks source link

Issue20 courses #37

Closed goldlilys closed 9 years ago

goldlilys commented 9 years ago

Updating html and casper test files. Add api test files and gulp update.

goldlilys commented 9 years ago

Fixed up the static html files and adding test validations for courses. Pulled master and merged fine because tests passes locally.

goldlilys commented 9 years ago

@treyhunner Should I remove the gulpfile.js from my commit or recommit without the gulp file? I only saw jeffrey's commit when I already pushed my changes. What do you recommend to fix it?

treyhunner commented 9 years ago

@goldlilys I'm not sure what would be best. Where did that gulpfile code come from?

jeffreytu commented 9 years ago

There are some errors for me when I run your api test.

Failures:

  1) Frisby Test: Enforce mandatory type must include -video, WBT, instructor led-
when creating
        [ POST http://localhost:3000/api/courses/ ]
   Message:
     Expected 200 to equal 422.
   Stacktrace:
     Error: Expected 200 to equal 422.
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:493:42)
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1074:43)
    at Timer.listOnTimeout [as ontimeout](timers.js:110:15)

  2) Frisby Test: Enforce mandatory type must include -video, WBT, instructor led-
when creating
        [ POST http://localhost:3000/api/courses/ ]
   Message:
     Error: Expected 'undefined' to be type 'object' for comparison
   Stacktrace:
     Error: Expected 'undefined' to be type 'object' for comparison
    at _jsonContains (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1238:11)
    at _jsonContains (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1250:9)
    at jasmine.Matchers.toContainJson (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1186:12)
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:717:24)
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1074:43)

  3) Frisby Test: Enforce mandatory unique title when creating
        [ POST http://localhost:3000/api/courses/ ]
   Message:
     Expected 200 to equal 422.
   Stacktrace:
     Error: Expected 200 to equal 422.
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:493:42)
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1074:43)
    at Timer.listOnTimeout [as ontimeout](timers.js:110:15)

  4) Frisby Test: Enforce mandatory unique title when creating
        [ POST http://localhost:3000/api/courses/ ]
   Message:
     Error: Expected 'undefined' to be type 'object' for comparison
   Stacktrace:
     Error: Expected 'undefined' to be type 'object' for comparison
    at _jsonContains (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1238:11)
    at _jsonContains (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1250:9)
    at jasmine.Matchers.toContainJson (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1186:12)
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:717:24)
    at null. (C:\Users\Jstadragon\Desktop\starters_academy\fullstack-project-01\node_modules\frisby\lib\frisby.js:1074:43)

Finished in 0.375 seconds
15 tests, 68 assertions, 4 failures, 0 skipped
goldlilys commented 9 years ago

@jeffreytu It depends on how your courses.spec.js and if you have the latest codes ... because on my courses.spec.js some are commented out for a reason since they're not supposed to run at the same time as the others. Did you make changes to that file while testing?

jeffreytu commented 9 years ago

If you are commenting out code then you should probably just take it out. Also, since we want a green build, your test would fail if merged into master. Each of us shouldn't be commenting out code to make your test pass. @treyhunner @tbashor any advice on this?

goldlilys commented 9 years ago

Removing the extra models that will be pushed by others and only adding the simple cases for tests to pass. Changed fullstack to db for model-config.json so it works with master.

Waiting till all of the fix-linter-errors branch is merged with master to continue running travis.

jeffreytu commented 9 years ago

Tests all pass for me. Was able to CRUD. Though, since you took out the mock models for the relations, of course I can't test them. I think the mock models should be left in all the way to merging. They don't need to be deleted before you merge them. What should happen is the person who did the actual models will replace your mock models when merging. @treyhunner @tbashor want to chime in for best practice advice?

goldlilys commented 9 years ago

@Rauld1 not sure, i thought that was default. Hmm now that you mention it, is there a difference? Because when I created my model, I did the default and it made that public to true

jeffreytu commented 9 years ago

I was reviewing your branch and everything seems good except when I do gulp test:all I get:

.POST /api/courses/ 422 5.778 ms - 1960
.POST /api/courses/ 422 5.346 ms - 1851
.POST /api/courses/ 422 4.121 ms - 1889
.

22 specs, 0 failures
Finished in 0 seconds
[22:49:54] Finished 'test:api' after 605 ms
[22:49:54] Starting 'test:integration'...
POST /api/courses/ 422 7.507 ms - 1853
[22:49:54] Finished 'test:integration' after 92 ms
[22:49:54] Starting 'server:stop'...
[22:49:54] Development server was stopped. (PID:7880)
[22:49:54] Finished 'server:stop' after 23 ms
[22:49:54] Finished 'test:all' after 4.07 s

Jstadragon@JSTADRAGON-S ~/Desktop/starters_academy/fullstack-project-01 (iss20-review)
$ Test file: C:/Users/Jstadragon/Desktop/starters_academy/fullstack-project-01/spec/integration/about_instructor_test.js
# page 1 navigates to page 2
FAIL About Instructors title good

I think this was discussed before where the development server is stopping before the casper tests run. We needed a callback. Don't think we fixed this in master then? I guess mine would have this error too.

Other than this, running gulp test:integration by itself, all tests pass. Checked merge master and it's good to go.

goldlilys commented 9 years ago

@jeffreytu I cloned this branch again to another directory in my local and ran gulp test:all , but everything seems to be running fine including casper tests. Are you still having problems with it? Probably need another person to test it out. If you're getting the same results as jeff, not sure what could be going wrong.

Rauld1 commented 9 years ago

I ran gulp on this branch and everything tested fine for me.

Sent from my iPhone

On Jan 31, 2015, at 11:20 PM, Frances Naty Go notifications@github.com wrote:

@jeffreytu I cloned this branch again to another directory in my local and ran gulp test:all , but everything seems to be running fine including casper tests. Are you still having problems with it?

— Reply to this email directly or view it on GitHub.

jeffreytu commented 9 years ago

It does the same thing on my #42 branch with master merged. Another Windows inconsistency? Can an instructor take a look at this?

goldlilys commented 9 years ago

@jeffreytu How weird!! test:integration works, but not test:all ? Aren't they calling the same task?

jeffreytu commented 9 years ago

Yeah. If I slc run and gulp test:integration it runs and passes.

treyhunner commented 9 years ago

I believe @jeffreytu and @Rauld1 gave this a +1 so I've merged it.

Tip: for a more clear +1 in the future use an emoji like this:

:+1: which makes a thumbs up: :+1: