Skip to content

Conversation

jkmassel
Copy link
Contributor

@jkmassel jkmassel commented May 27, 2024

Adds tests for #119 – the code works correctly, and this does overlap with what the Link Header Parser's test suite should do, but now that we're re-mapping the output in a more efficient way I figure it'd be nice to be able to quickly ensure that it's working.

To Test:

  • Ensure CI passes

@jkmassel jkmassel requested a review from oguzkocer May 27, 2024 22:25
@jkmassel jkmassel self-assigned this May 27, 2024
@jkmassel jkmassel marked this pull request as ready for review May 27, 2024 22:26

[dev-dependencies]
chrono = { version = "0.4" }
claim = "0.5"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably not worth adding a dependency just for assert_none which can be done with .is_none().

Comment on lines 171 to 204
#[test]
fn test_link_header_can_be_parsed_for_first_page() {
let response = WPNetworkResponse{ body: Vec::with_capacity(0), status_code: 200, header_map: Some(HashMap::from([("Link".to_string(), "<http://localhost/wp-json/wp/v2/posts?page=2>; rel=\"next\"".to_string())])) };
assert_eq!(response.get_link_header("next").unwrap(), Url::parse("http://localhost/wp-json/wp/v2/posts?page=2").unwrap());
assert_none!(response.get_link_header("prev"));
}

#[test]
fn test_link_header_can_be_parsed_for_intermediate_pages() {
let response = WPNetworkResponse{ body: Vec::with_capacity(0), status_code: 200, header_map: Some(HashMap::from([("Link".to_string(), "<http://localhost/wp-json/wp/v2/posts?page=1>; rel=\"prev\", <http://localhost/wp-json/wp/v2/posts?page=3>; rel=\"next\"".to_string())])) };
assert_eq!(response.get_link_header("prev").unwrap(), Url::parse("http://localhost/wp-json/wp/v2/posts?page=1").unwrap());
assert_eq!(response.get_link_header("next").unwrap(), Url::parse("http://localhost/wp-json/wp/v2/posts?page=3").unwrap());
}

#[test]
fn test_link_header_can_be_parsed_for_last_page() {
let response = WPNetworkResponse{ body: Vec::with_capacity(0), status_code: 200, header_map: Some(HashMap::from([("Link".to_string(), "<http://localhost/wp-json/wp/v2/posts?page=5>; rel=\"prev\"".to_string())])) };
assert_none!(response.get_link_header("next"));
assert_eq!(response.get_link_header("prev").unwrap(), Url::parse("http://localhost/wp-json/wp/v2/posts?page=5").unwrap());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these tests can be written in a much nicer way with rstest that'll allow us to add more cases without having to copy/paste the implementation.

Since there were some syntax suggestions as well, I combined all of them into a suggestion commit: 53e7072. Let me know what you think!

@jkmassel jkmassel force-pushed the add/link-header-tests branch 2 times, most recently from 141b035 to 81d80cb Compare May 29, 2024 02:30
@jkmassel jkmassel requested a review from oguzkocer May 29, 2024 02:51
@oguzkocer oguzkocer force-pushed the add/link-header-tests branch from 81d80cb to f50c3eb Compare May 29, 2024 04:14
@oguzkocer oguzkocer enabled auto-merge (squash) May 29, 2024 04:14
@oguzkocer oguzkocer merged commit b64724f into trunk May 29, 2024
@oguzkocer oguzkocer deleted the add/link-header-tests branch May 29, 2024 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants