stablize occasional failed tests

[ramsayleung]

Trying to fix failed tests.

[ramsayleung]

The unit tests of rspotify mostly depending on the web api of Spotify, and sometimes the tests will fail become of network situation, rate limit of Spotify api, or other reason. I am trying to make the tests more stable by run them repeatedly, if any call of them passes, it means the test is fine. The example code of repeated call:

async fn test_albums() {
    let any_success = Arc::new(Mutex::new(false));
    let mut handlers = Vec::new();
    // run function for 50 times, pass if any call of all is ok.
    for _ in 0..50 {
        let any_success = Arc::clone(&any_success);
        let handler = tokio::spawn(async move {
            let spotify = Spotify::default()
                .client_credentials_manager(CLIENT_CREDENTIAL.lock().unwrap().clone())
                .build();
            let birdy_uri1 = String::from("spotify:album:41MnTivkwTO3UUJ8DrqEJJ");
            let birdy_uri2 = String::from("spotify:album:6JWc4iAiJ9FjyK0B59ABb4");
            let birdy_uri3 = String::from("spotify:album:6UXCm6bOO4gFlDQZV5yL37");
            let track_uris = vec![birdy_uri1, birdy_uri2, birdy_uri3];
            let albums = spotify.albums(track_uris).await;
            let mut pass = any_success.lock().unwrap();
            *pass |= albums.is_ok();
        });
        handlers.push(handler);
    }
    futures::future::join_all(handlers).await;
    let pass = any_success.lock().unwrap();
    assert!(*pass)
}

The core logic of the test is:

let spotify = Spotify::default()
    .client_credentials_manager(CLIENT_CREDENTIAL.lock().unwrap().clone())
    .build();
let birdy_uri1 = String::from("spotify:album:41MnTivkwTO3UUJ8DrqEJJ");
let birdy_uri2 = String::from("spotify:album:6JWc4iAiJ9FjyK0B59ABb4");
let birdy_uri3 = String::from("spotify:album:6UXCm6bOO4gFlDQZV5yL37");
let track_uris = vec![birdy_uri1, birdy_uri2, birdy_uri3];
let albums = spotify.albums(track_uris).await;

and other statement is redundant, and I am trying to refactor the repeated test as an async function, pass the core logic function as a function parameter:

// F is a async function
async fn repeated_test<F>(f: F) {
    let any_success = Arc::new(Mutex::new(false));
    let mut handlers = Vec::new();
    // run function for 50 times, pass if any call of all is ok.
    for _ in 0..50 {
        let any_success = Arc::clone(&any_success);
        let handler = tokio::spawn(async move {
            let success = f().await;
            let mut pass = any_success.lock().unwrap();
            *pass |= success.is_ok();
        });
        handlers.push(handler);
    }
    futures::future::join_all(handlers).await;
    let pass = any_success.lock().unwrap();
    assert!(*pass)
}

async fn test_albums() {
    repeated_test(|| {
        let spotify = Spotify::default()
            .client_credentials_manager(CLIENT_CREDENTIAL.lock().unwrap().clone())
            .build();
        let birdy_uri1 = String::from("spotify:album:41MnTivkwTO3UUJ8DrqEJJ");
        let birdy_uri2 = String::from("spotify:album:6JWc4iAiJ9FjyK0B59ABb4");
        let birdy_uri3 = String::from("spotify:album:6UXCm6bOO4gFlDQZV5yL37");
        let track_uris = vec![birdy_uri1, birdy_uri2, birdy_uri3];
        let albums = spotify.albums(track_uris).await;
        return albums.is_ok();
    });
}

Because the lifetime and ownership of async function is too complex , so that I have no idea how to pass async function as a parameter.

[Rigellute]

I've not tried this, but this might work?

async fn repeated_test<F>(f: F) where F: Future + Send + 'static {
  ...
}

And then use the function with async move

 repeated_test(async move || {
   ...
});

[marioortizmanero]

Are repeated tests the way to go really? If anything you're testing the Spotify's rate limit, I don't see how running the function multiple times could be useful for our tests. If the first test fails, the next ones also will, because you've already broken the rate limit with the first test.

I doubt it's an internal issue with Spotify's API, because other libraries don't deal with this, as I pointed out in #110.

[ramsayleung]

If anything you're testing the Spotify's rate limit, I don't see how running the function multiple times could be useful for our tests. If the first test fails, the next ones also will, because you've already broken the rate limit with the first test.

Yes, you make your point. So I am still thinking about how to make these test cases more stable. IMHO, I think the key point is these test cases are depending on a 3rd-party service, testing of integration with 3rd-party services should use mock data, but rspotify is a Web API wrapper of Spotify, there is no way to mock Spotify's Web API.