Skip to content

test: add failing test for selfavatar #135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

Conversation

ralphtheninja
Copy link
Collaborator

No description provided.

}

0
pub unsafe fn dc_is_blobdir_path(context: &Context, path: *const libc::c_char) -> bool {
Copy link
Collaborator Author

@ralphtheninja ralphtheninja Jun 7, 2019

Choose a reason for hiding this comment

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

Nothing to do with these tests, but it came in my way and was easy to fix.

let image_c = CString::new(image_path_2.to_str().unwrap()).unwrap();
let abs_path = unsafe { dc_get_abs_path(&ctx.ctx, image_c.as_ptr()) };

assert_eq!(to_string(abs_path), image_path.to_str().unwrap());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test works on windows and *nix.

let mut image_path = PathBuf::from(to_string(blobdir_c));
image_path.push("delta-logo.png");

assert_eq!(to_string(selfavatar), image_path.to_str().unwrap());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should fail on windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

no totally sure, but dc_set_config() tries to copy the given file physically to the blob directory. i assume the file described by logo_path_c normally does not exist? maybe the behavior of copying unexistent files is slightly different at some level (dc_set_config() uses the source file when copying failed)

to have a reliable test, maybe ensure that the given file exists before calling dc_get_config()? no need to be a valid image, an empty file should do the job.

Copy link
Collaborator Author

@ralphtheninja ralphtheninja Jun 9, 2019

Choose a reason for hiding this comment

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

to have a reliable test, maybe ensure that the given file exists before calling dc_get_config()? no need to be a valid image, an empty file should do the job.

That's what the test is doing, logo_path_c always points to an existing file in the fixtures folder.

@ralphtheninja
Copy link
Collaborator Author

ralphtheninja commented Jun 7, 2019

Rebase after #134 has been merged and fix to_str() -> as_str()

Nevermind, I was using to_string().

@r10s
Copy link
Contributor

r10s commented Jun 7, 2019

had a look at the logic of calling dc_is_blobdir_path(), lgtm

@ralphtheninja
Copy link
Collaborator Author

Need #136 merged so I can use the string conversions @flub implemented.

@ralphtheninja
Copy link
Collaborator Author

@@ -966,6 +966,7 @@ fn test_selfavatar_config() {
logo_path.push("tests");
logo_path.push("fixtures");
logo_path.push("delta-logo.png");
assert!(logo_path.as_path().exists());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is only asserting that we do have a real file in the test.

@@ -987,6 +988,7 @@ fn test_selfavatar_config() {
let mut image_path = PathBuf::from(to_string(blobdir_c));
image_path.push("delta-logo.png");

assert!(image_path.as_path().exists());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The resulting file in the blobdir should exist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems windows manages to copy the file to the blobdir at least. This is good to know.

@ralphtheninja
Copy link
Collaborator Author

Side note: I've tested this on desktop and it's not a problem there since the browser doesn't care about mixed / or \.

@hpk42
Copy link
Contributor

hpk42 commented Jul 24, 2019

retiring this PR as there is no activity but made a note at #116 that this PR exists.

@hpk42 hpk42 closed this Jul 24, 2019
phranck pushed a commit to open-xchange/deltachat-core-rust that referenced this pull request Sep 16, 2019
@hpk42 hpk42 deleted the selfavatar branch February 19, 2020 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants