Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conforming to current jasmine api. Passing needed test values #42

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Conversation

ryus08
Copy link
Contributor

@ryus08 ryus08 commented Apr 9, 2018

I started to implement #41 and wanted to see if the existing tests still passed, but they weren't running.

Wanted to get them working for before adding the feature. Just a few things off, comments inline.

@@ -1,5 +1,5 @@
var Jasmine = require('jasmine');
var SpecReporter = require('jasmine-spec-reporter');
var { SpecReporter } = require('jasmine-spec-reporter');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jasmine-spec-reporter has a couple objects. Dunno if it always did

@@ -5,7 +5,7 @@
"An API to help facilitate the use of the Yahoo! Fantasy Sports API in NodeJS projects.",
"main": "index.js",
"scripts": {
"test": "node -r @std/esm jasmine-runner.js",
"test": "node -r esm jasmine-runner.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scoped package @std/esm is not currently in the package.json, but plain esm is. Could alternatively switch the package.json to @std/esm.

@@ -67,13 +67,13 @@ describe("resource : gameResource", function() {
});

beforeEach(function() {
spyOn(yf, "api").andCallThrough();
spyOn(yf, "api").and.callThrough();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that jasmine changed their API in version 2. Version 3 is specified in the package.json, so I think we need this syntax.

});

// leagues
describe(": leagues", function() {
it("should build a proper url to retrieve league data for a single league using a numeric game key", function() {
game.leagues(328, "328.l.34014", null);
game.leagues(328, "328.l.34014", () => {});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need to provide some callback, even though we aren't testing the value called-back-with.

});

// meta
it ("should build a proper url to retrieve metadata via a league key", function() {
nock('https://fantasysports.yahooapis.com')
.get("/fantasy/v2/league/328.l.34014/metadata?format=json")
.reply(200, {});
.reply(200, require("./nock-data/leagueMeta").meta);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to return some data, because the object mapping classes expect there to be at least a fantasy_content field.

.get("/fantasy/v2/league/328.l.34014/players?format=json")
.reply(200, {});
// // players
// it ("should build a proper url to retrieve players via a league key", function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is commented out in the source, so there is nothing to test yet.

@whatadewitt
Copy link
Owner

I'll take a look at this tonight... funny thing is I actually made a bunch of changes locally but didn't merge them in yet as I still had a few things breaking... thanks!

nock("https://fantasysports.yahooapis.com")
.get("/fantasy/v2/league/328.l.34014/players?format=json")
.reply(200, {});
// // players
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was commented out in the source, so there is nothing to test

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya the tests are a never ending process :P

@ryus08
Copy link
Contributor Author

ryus08 commented Apr 10, 2018

Yeah, those changes you had were some of it. Still need to nock out some endpoints and provide a callback

@@ -51,9 +51,9 @@ describe("resource: leagueResource", function() {
it("should build a proper url to retrieve metadata via a league key", function() {
nock("https://fantasysports.yahooapis.com")
.get("/fantasy/v2/league/328.l.34014/metadata?format=json")
.reply(200, {});
.reply(200, require("./nock-data/leagueMeta").meta);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to return some data, because the object mapping classes expect there to be at least a fantasy_content field.

@ryus08 ryus08 reopened this Apr 12, 2018
@whatadewitt whatadewitt merged commit a81baa2 into whatadewitt:master Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants