Looking for a maintainer to help with the test suite here / maintain?

[lwasser]

Hey @hozn please say the word if this is not something you want. I am now leading pyopensci. i wondered if i can help you find a new maintainer(s) through that effort - at a minimum i think we need a test suite that works on CI. i'm willing to help but don't feel confident with api based test suites as mocking / monkey patching is kind of an art and one that I dont want to abuse.

[yihong0618]

I can help

[lwasser]

that would be awesome. @hozn i think i know the answer to this but are you ok with some help on refactoring the test suite so we can setup CI for it?

[lwasser]

i might also be able to find a few others on twitter through my organization, pyopensci.

[jsamoocha]

Part of the current test suite depends on an existing Strava account/API key. Mocking that part may require quite some (re-)work - many tests assume particular data to be there in Strava.

Maybe the first step for CI inclusion would be to start with only the unit tests since these are independent of the environment. After that, we can gradually add the existing functional/integration tests by mocking the Strava API.

Before proceeding, maybe we should think first about what exactly we want to test (based on coverage and priorities).

[hozn]

@lwasser , I appreciate the help you've provided on this project. Finding a new maintainer would be great. @yihong0618 if you're interested, I'm happy to set you up with perms (or @lwasser can do same, I believe).

@jsamoocha , agree that the functional tests aren't going to be easy to get running in CI. I really wish strava would provide a key that could be used with test data for these sorts of purposes. The biggest source of issues in the library is strava's continually changing API. Usually that is just adding (and removing) attributes. Hard to test without actual API access, though.

[yihong0618]

@hozn Thanks my pleasure.

I have made some repos based on strvalib like https://github.com/yihong0618/running_page and https://github.com/yihong0618/GitHubPoster

Kind of having a little knowledge stravalib source code.

I can do some code review, merge and refactor jobs. @lwasser what do you think.

[lwasser]

@yihong0618 this all sounds great. I don't think I can add @yihong0618 as a maintainer @hozn so if you can do that - it would be great. And i really love the idea of starting with unit tests and slowly adding tests to hit the API as it makes sense.

One with with a mock is it would / could atleast detect a change in what strava returns but i get that testing the API is really hard as well. so @hozn please give permissions and @yihong0618 i am happy to provide input on test updates and refactoring but would look to you to do the actual implementation :)

i'm also happy to update docs as they need to be updated. thank you!!

[jsamoocha]

Is it necessary to test against the real/mocked Strava API? I can see (but please correct me if I'm wrong) the following approach working for this package:

Strava API additions: Someone will want to use a new endpoint or attribute and will eventually submit an issue/PR. We'll need to make sure (through code review or even automated) it is properly documented and added to the Sphinx autodoc magic.

Strava API updates: These will eventually lead to issues/PR's as above.

Strava endpoint removal: These will show up in CI as failed link checks in the documentation build (which happened to me today).

All changes in the Strava API that don't lead to issues/PR's seem to have no impact on users of this package. Proactive testing, in this case, may even only lead to delayed merging of unrelated PR's because of failed CI.

[hozn]

@lwasser , will do! (And, yes, welcome any refactoring!)

[jsamoocha]

To proactively check for changes in the Strava API (which I doubt should be part of CI), we could use the swagger API reference and some automated inspection of our own domain model and endpoints. It even has example responses we could use for testing happy paths (without having to write all te tests by hand).

[yihong0618]

@hozn @lwasser just received the invitation thank you very much. will do my best.

[yihong0618]

@hozn @lwasser I think we also need to add one GitHub Actions workflow to upload the package to PyPI

[jsamoocha]

I have some time to help with the following on a short term:

• Add a Github Action workflow for the unit tests
• Review the existing functional/integration tests and move all tests that are independent from the Strava API to the unit test package

@yihong0618 @lwasser please let me know if you agree with this approach and it doesn't overlap with your efforts

[yihong0618]

@jsamoocha cool. But I think we can not add secret @hozn for this repo as Collaborators if we use current way for unit tests we need add some secrets IMO

[jsamoocha]

I don't think the unit tests require any secrets. I can run them locally without problems. Some tests in the functional test package depend on a (personal) Strava account; these could be left out of CI for the time being?

[yihong0618]

that will be nice, I think unit tests is enough for now, personal things we could test locally

[lwasser]

Ok y'all - this is awesome. what do you think about opening up several issues here to track all of the work. i will do that and assign each of us tasks. that way we can discuss each separately!

Here is a list of things that I think we need to work on

1. Upload to pypi. - a release workflow. I"m happy to work on this. Are you comfortable with my adding something like bump version to update versions in docs and throughout the package. Then we can really focus on semantic versioning We can then add an action to push to pypi on release. I would open an issue seeing if @hozn is comfortable with sharing pypi credentials with me and adding me as a full admin or giving me super powers to this repo to add secrets and such as people need to make those types of changes. i'm happy to oversee that! @lwasser can work on this . I will also have a look at how change log is being handled as i do this and try to really clean up and document the release workflow!
2. Update test suite:
   • Create a new test suit that runs on CI. it sounds like for now we will focus on unit tests only and will keep tests on the API local. @yihong0618 it sounds like you'd like to work on this?
   • Add a CI build for that test suite - sounds like @jsamoocha can work on this

I think doing the above things will make is REALLY much easier for others to contribute too.

---

Colleagues - i do think we should consider some CI based tests for the api in the FUTURE or something that tests API functionality. i've seen things done with casettes for instance where we could record responses that could build on CI and update those periodically locally. but let's focus on what we can do now first to make it easier for others to contribute :) that can be a separate discussion in another issue! super psyched that y'all are willing to help here!!

Does this all sound good? if so i'll open a few issues and will request more permissions from @hozn to oversee this new dev work!

[yihong0618]

sounds great. Thank you.

[jsamoocha]

Yeah, sounds good. I'll refactor the existing tests and add a CI workflow for these. If given the permissions to do so, I can also act as a backup for @yihong0618 for reviewing/merging PR's.

[lwasser]

hey @yihong0618 @jsamoocha i wanted to check back in here! @hozn setup stravalib here in a new org! this will make it easier to give teams permissions to work on stravalib. I also can help wtih that!

I think Hans also added me to pypi - i need to check that. Once i have a bit of time I can work on the build upon release workflow via a github action.

In the meantime, what type of permissions do y'all need to get started here?

I have the following on my list:

1. Setup pypi release /github action
2. Update readme with badges for this and probably update doc workflow on read the docs given the migration to this org!
3. Create issues that clarify who is doing what based upon this issue!!

Let me know if you need different permissions or if you need anything to get started! 🎆

[jsamoocha]

Hi @lwasser ,

If @yihong0618 and I could have write permissions (or something more fine-grained just to allow us to merge PRs), we would be less dependent on your and @hozn 's time availability for merges.

I'm making some progress in modernizing the test suite (see #245), so that doesn't need a separate issue anymore.

What remains is IMO thinking about if and how we're going to test our "compliance" to the Strava API (e.g., by checking if all declared endpoints have an implementation in stravalib). This could be a scheduled job that acts à la dependabot. We could have this discussion in a separate issue if you want to.

[hozn]

@lwasser , I'll defer to you to grant write perms, as you're the one actively maintaining this, but certainly support more folks having write perms to the repo!

[lwasser]

ok awesome @hozn i'll go ahead and get everyone maintainer level permissions to be able to handle pr's !! :)thanks for chiming in here Hans so I know this is ok!!

[lwasser]

Ok @yihong0618 @jsamoocha i've provided both with write access vis a maintainer team that i created. I could bump you to maintainers if you need that level of access - please let me know.! You should be able to merge pr's. i'm going to lock down the main branch to avoid direct pushes to main without a PR.

it think it's nice to have issues for things just to track things that have been completed in pr's with issues that describe the goal of that item, etc. especially if we implement a change log update for releases and patches and such!

[jsamoocha]

Ok, I made an issue describing the test/CI refactoring work; see #246

[lwasser]

hi colleagues! i believe that we have sorted out the maintainer issue at this point . i'm just going through and cleaning up old issues to get a sense of how many unaddressed issue we have here. closing this but please do reopen if you feel like there is more discussion required!