Repository: stravalib/stravalib
Pull request #256 (2022-11-15 19:58:43)

Test client - upload_activity
Fully tests upload_activity(). (part of #250)

I could use some advice on if/how to use `FutureWarning`s in case of seemingly deprecated API parameters.


Patch:
diff --git a/stravalib/client.py b/stravalib/client.py
index 9ebc9a5..2568362 100644
--- a/stravalib/client.py
+++ b/stravalib/client.py
@@ -4,12 +4,13 @@ Client
 Provides the main interface classes for the Strava version 3 REST API.
 """
 
+import json
 import logging
-# import warnings # unused import
 import functools
 import time
 import collections
 import calendar
+import warnings
 from io import BytesIO
 from datetime import datetime, timedelta
 
@@ -24,6 +25,10 @@ from stravalib.util import limiter
 from stravalib import unithelper
 
 
+warnings.simplefilter('default')
+logging.captureWarnings(True)
+
+
 class Client(object):
     """
     Main client class for interacting with the exposed Strava v3 API methods.
@@ -233,9 +238,7 @@ class Client(object):
                                       result_fetcher=result_fetcher,
                                       limit=limit)
 
-
-    # TODO: Doble check these endpoint doc URLs are correct
-    def get_athlete(self, athlete_id=None):
+    def get_athlete(self):
         """
         Gets the specified athlete; if athlete_id is None then retrieves a
         detail-level representation of currently authenticated athlete;
@@ -248,14 +251,7 @@ class Client(object):
         :return: The athlete model object.
         :rtype: :class:`stravalib.model.Athlete`
         """
-        if athlete_id is None:
-            raw = self.protocol.get('/athlete')
-        else:
-            raise NotImplementedError("The /athletes/{id} endpoint was removed by Strava.  "
-                                      "See https://developers.strava.com/docs/january-2018-update/")
-
-            # raw = self.protocol.get('/athletes/{athlete_id}', athlete_id=athlete_id)
-
+        raw = self.protocol.get('/athlete')
         return model.Athlete.deserialize(raw, bind_client=self)
 
     # TODO: this endpoint was removed so do we want to remove the URL altogether?
@@ -663,7 +659,8 @@ class Client(object):
         return model.Activity.deserialize(raw_activity, bind_client=self)
 
     def upload_activity(self, activity_file, data_type, name=None, description=None,
-                        activity_type=None, private=None, external_id=None):
+                        activity_type=None, private=None, external_id=None, trainer=None,
+                        commute=None):
         """
         Uploads a GPS file (tcx, gpx) to create a new activity for current athlete.
 
@@ -686,13 +683,24 @@ class Client(object):
                               nordicski, alpineski, backcountryski, iceskate, inlineskate,
                               kitesurf, rollerski, windsurf, workout, snowboard, snowshoe
                               Type detected from file overrides, uses athlete's default type if not specified
+                              WARNING - This param is supported (as of 2022-11-15), but not
+                              documented and may be removed in the future.
         :type activity_type: str
 
-        :param private: (optional) set to True to mark the resulting activity as private, 'view_private' permissions will be necessary to view the activity
+        :param private: (optional) set to True to mark the resulting activity as private,
+                        'view_private' permissions will be necessary to view the activity.
+                        WARNING - This param is not supported by the Strava API and may be
+                        removed in the future.
         :type private: bool
 
         :param external_id: (optional) An arbitrary unique identifier may be specified which will be included in status responses.
         :type external_id: str
+
+        :param trainer: (optional) Whether the resulting activity should be marked as having been performed on a trainer.
+        :type trainer: bool
+
+        :param commute: (optional) Whether the resulting activity should be tagged as a commute.
+        :type commute: bool
         """
         if not hasattr(activity_file, 'read'):
             if isinstance(activity_file, str):
@@ -712,13 +720,29 @@ class Client(object):
         if description is not None:
             params['description'] = description
         if activity_type is not None:
+            warnings.warn(
+                'The "activity_type" parameter is undocumented in the Strava API. Its use '
+                'may lead to unexpected behavior or errors in the future.',
+                FutureWarning,
+                stacklevel=2
+            )
             if not activity_type.lower() in [t.lower() for t in model.Activity.TYPES]:
                 raise ValueError("Invalid activity type: {0}.  Possible values: {1!r}".format(activity_type, model.Activity.TYPES))
             params['activity_type'] = activity_type
         if private is not None:
+            warnings.warn(
+                'The "private" parameter is unsupported by the Strava API. It has no '
+                'effect and may lead to errors in the future',
+                DeprecationWarning,
+                stacklevel=2
+            )
             params['private'] = int(private)
         if external_id is not None:
             params['external_id'] = external_id
+        if trainer is not None:
+            params['trainer'] = json.dumps(trainer)
+        if commute is not None:
+            params['commute'] = json.dumps(commute)
 
         initial_response = self.protocol.post('/uploads',
                                               files={'file': activity_file},


Test patch:
diff --git a/stravalib/tests/integration/test_client.py b/stravalib/tests/integration/test_client.py
index 024d39f..d12d2af 100644
--- a/stravalib/tests/integration/test_client.py
+++ b/stravalib/tests/integration/test_client.py
@@ -32,7 +32,76 @@ def test_get_activity(mock_strava_api, client, include_all_efforts, expected_url
     assert activity.id == test_activity_id
 
 
-def test_upload_activity(mock_strava_api, client):
+@pytest.mark.parametrize(
+    'activity_file_type,data_type,name,description,activity_type,private,external_id,'
+    'trainer,commute,expected_params,expected_exception',
+    (
+        ('file', 'tcx', None, None, None, None, None, None, None, {'data_type': 'tcx'}, None),
+        ('str', 'tcx', None, None, None, None, None, None, None, {'data_type': 'tcx'}, None),
+        ('not_supported', 'tcx', None, None, None, None, None, None, None, None, TypeError),
+        ('file', 'invalid', None, None, None, None, None, None, None, None, ValueError),
+        ('file', 'tcx', 'name', None, None, None, None, None, None, {'data_type': 'tcx', 'name': 'name'}, None),
+        ('file', 'tcx', None, 'descr', None, None, None, None, None, {'data_type': 'tcx', 'description': 'descr'}, None),
+        ('file', 'tcx', None, None, 'Run', None, None, None, None, {'data_type': 'tcx', 'activity_type': 'Run'}, None),
+        ('file', 'tcx', None, None, 'run', None, None, None, None, {'data_type': 'tcx', 'activity_type': 'run'}, None),
+        ('file', 'tcx', None, None, 'sleep', None, None, None, None, None, ValueError),
+        ('file', 'tcx', None, None, None, True, None, None, None, {'data_type': 'tcx', 'private': '1'}, None),
+        ('file', 'tcx', None, None, None, None, 42, None, None, {'data_type': 'tcx', 'external_id': '42'}, None),
+        ('file', 'tcx', None, None, None, None, None, True, None, {'data_type': 'tcx', 'trainer': 'true'}, None),
+        ('file', 'tcx', None, None, None, None, None, None, True, {'data_type': 'tcx', 'commute': 'true'}, None)
+    )
+)
+def test_upload_activity(
+        mock_strava_api,
+        client,
+        activity_file_type,
+        data_type,
+        name,
+        description,
+        activity_type,
+        private,
+        external_id,
+        trainer,
+        commute,
+        expected_params,
+        expected_exception
+):
+    init_upload_response = {
+        'id': 1,
+        'id_str': 'abc',
+        'external_id': 'abc',
+        'status': 'default_status',
+        'error': ''
+    }
+    upload_kwargs = {
+        **({'name': name} if name is not None else {}),
+        **({'description': description} if description is not None else {}),
+        **({'activity_type': activity_type} if activity_type is not None else {}),
+        **({'private': private} if private is not None else {}),
+        **({'external_id': external_id} if external_id is not None else {}),
+        **({'trainer': trainer} if trainer is not None else {}),
+        **({'commute': commute} if commute is not None else {})
+    }
+
+    def _call_upload(*args, **kwargs):
+        if expected_exception:
+            with pytest.raises(expected_exception):
+                client.upload_activity(*args, **kwargs)
+        else:
+            mock_strava_api.post('/uploads', status=201, json=init_upload_response)
+            _ = client.upload_activity(*args, **kwargs)
+            assert mock_strava_api.calls[-1].request.params == expected_params
+
+    with open(os.path.join(RESOURCES_DIR, 'sample.tcx')) as f:
+        if activity_file_type == 'file':
+            _call_upload(f, data_type, **upload_kwargs)
+        elif activity_file_type == 'str':
+            _call_upload(f.read(), data_type, **upload_kwargs)
+        else:
+            _call_upload({}, data_type, **upload_kwargs)
+
+
+def test_activity_uploader(mock_strava_api, client):
     test_activity_id = 42
     init_upload_response = {
         'id': 1,