Finish replacing real API calls with mocks in test suite#183
Conversation
- Convert the existing test for listZones() from a skipped test to an active one. - Mock the API response for heart rate and power zones. - Add assertions to verify the structure and values of the returned zones, including distribution buckets. - Ensure the test checks both power and heartrate zones for expected properties.
This method doesn't actually exist in the library, only the typescript definitions and a skipped test exist. There is also no corollary in the strava api documentation. - Deleted the listPhotos method from the ActivitiesRoutes interface in index.d.ts. - Removed the associated test case for listPhotos from activities.js, which was previously marked as skipped.
All tests are written and mocked! - Removed several functions from testsHelper that were designed to fetch sample data from the Strava API, including getSampleAthlete, getSampleActivity, getSampleClub, getSampleRoute, getSampleGear, getSampleSegmentEffort, getSampleSegment, and getSampleRunningRace. - This cleanup reduces code complexity and focuses on essential helper functions.
- Replaced Travis CI badge with GitHub Actions badge. - Clarified test instructions, emphasizing that all tests now use `nock` to mock the Strava API. - Enhanced the description of the test suite's functionality and requirements. - Updated maintainer information to include Wesley Schlenker.
📝 WalkthroughWalkthroughRemoved Drone and Travis CI configs, updated README (badges, endpoints, wording), bumped package version to 3.3.0, adjusted TypeScript definitions (added/removed methods), added Changes
Sequence Diagram(s)(omitted — changes do not introduce a new multi-component control flow needing sequence visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Added `starSegment` method to `SegmentsRoutes` interface in index.d.ts. - Introduced `route` method to `StreamsRoutes` interface in index.d.ts. - Removed references to `running_races` from README.md. - Updated README.md
listFollowers doesn't exist
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 325: Update the README description for strava.segments.explore(args,done)
to hyphenate the compound adjective: change "comma separated string" to
"comma-separated string" in the bounds argument description so it reads
`"bounds" as a comma-separated string, for two points describing a rectangular
boundary...` to improve grammar and consistency.
- Line 420: Update the README sentence "Rate limiting functionality is properly
tested" to use the hyphenated compound adjective "Rate-limiting functionality is
properly tested" so the compound adjective is grammatically correct; edit the
README.md content where that line appears.
The test is named `#updateSportType()` but calls `strava.activities.update()`. There is no `updateSportType()` method in the implementation. The test name suggests a different method.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/activities.js (1)
127-150: Duplicatedescribeblock name creates confusing test output.There are now two
describe('#update()', ...)blocks (lines 102–125 and 127–150). This will produce duplicate test group names in the output, making it harder to identify which test failed. Consider renaming this block to distinguish it from the other update test.Suggested fix
- describe('#update()', function () { + describe('#update() - sport type', function () { it('should update the sport type of an activity', async function () {
🧹 Nitpick comments (1)
test/activities.js (1)
154-208: Nock mock missing consistency with other tests in this file.All other mocks in this file include
.query(true),.matchHeader('authorization', /Bearer .+/), and.once(). This mock is missing these, which could mask issues with query parameter handling or authorization headers.Suggested fix for consistency
// Mock the list zones API call nock('https://www.strava.com') .get('/api/v3/activities/' + testActivity.id + '/zones') + .query(true) + .matchHeader('authorization', /Bearer .+/) + .once() .reply(200, [
markstos
left a comment
markstos
left a comment
There was a problem hiding this comment.
Reviewed diff. These all look like solid changes.
Moving over to 100% mocks sets up to run tests in CI.
2 participants
This PR completes the migration of the test suite to use mock API calls instead of the real Strava API.
Changes
listZones()test to use mocks (previously skipped)listPhotos()test (previously skipped)Testing
yarn testcompletes entirely without an apiKeySummary by CodeRabbit
Chores
Documentation
API Changes
Tests
Version: 3.3.0
✏️ Tip: You can customize this high-level summary in your review settings.