search

node-strava / node-strava-v3

API wrapper for Strava's v3 API, in Node
MIT License
356 stars 109 forks source link

Allow 0 as args value and add arg "context_entries" #79

Closed lavolp3 closed 5 years ago

lavolp3 commented 5 years ago

This is especially helpful when calling segment leaderboards. "per_page" = 0 and "context_entries" = 0 result in a single entry of the authenticated user. Includes changes of PR #71

markstos commented 5 years ago

I would prefer to have an automated test to go with these, but I read the diff, and the changes look trivial and clear, so merging anyway.

markstos commented 5 years ago

Before I release, could you be helpful and look into why the tests are failing? (It may not be related to your PR)

lavolp3 commented 5 years ago

I'm not sure how exactly Travis CI works but all fails result from a "Cannot read property X of undefined". So not related to my PR. I'm just guessing here: Is Travis CI building and testing the script without some important input like an athlete id?

markstos commented 5 years ago

There is a major new version coming tomorrow.

As part of preparing that, I was reminded that the legacy test suite is very difficult to contribute to. First, you have to go through a manual process to issue an access token for yourself. Then you need to run most tests against a live Strava account. Strava has removed the API method to delete activities, so you are guaranteed (I think) to end up with junk in the account. Finally, there are requirements that several types of data be pre-created in the account. The result is you need to sign up for a separate dummy Strava account and pre-fill it with various kinds of test data. It's a big pain.

There is one test that uses nock instead. It mocks the Strava API and solves all the problems above, making the tests easy and fast to run and contribute to. Over time, all tests should be updated to use nock instead of the current design.

I'll see about including this change in the new release as well.

On Sun, Sep 1, 2019 at 5:24 PM Dirk notifications@github.com wrote:

I'm not sure how exactly Travis CI works but all fails result from a "Cannot read property X of undefined". So not related to my PR. I'm just guessing here: Is Travis CI building and testing the script without some important input like an athlete id?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/UnbounDev/node-strava-v3/pull/79?email_source=notifications&email_token=AAAGJZJVPD63Q7H7ZG6KE6TQHQXJBA5CNFSM4IRBX2G2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5UK4KA#issuecomment-526954024, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAGJZO7DSQM24Y5OCQWFWTQHQXJBANCNFSM4IRBX2GQ .

-- Mark Stosberg Director of Systems and Security RideAmigos