Skip to content
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

2 tests fail on git master #253

Closed
tessus opened this issue Mar 6, 2024 · 9 comments · Fixed by #254
Closed

2 tests fail on git master #253

tessus opened this issue Mar 6, 2024 · 9 comments · Fixed by #254

Comments

@tessus
Copy link
Collaborator

tessus commented Mar 6, 2024

Steps to reproduce:

git clone --depth=1 https://github.com/orhun/rustypaste.git rp-test
cd rp-test
cargo test -- --test-threads 1

This is what I get on macOS and on Linux:

$ cargo test -- --test-threads 1
    Finished test [unoptimized + debuginfo] target(s) in 0.12s
     Running unittests src/lib.rs (target/debug/deps/rustypaste-0b6cbf8b54a0ce79)

running 36 tests
test auth::tests::test_extract_tokens ... ok
test config::tests::test_get_tokens ... ok
test config::tests::test_parse_config ... ok
test config::tests::test_parse_deprecated_config ... ok
test config::tests::test_space_handling ... ok
test file::tests::test_file_checksum ... ok
test header::tests::test_content_disposition ... ok
test header::tests::test_expiry_date ... ok
test mime::tests::test_match_mime_type ... ok
test paste::tests::test_paste_data ... FAILED
test random::tests::test_generate_url ... ok
test server::tests::test_auth ... ok
test server::tests::test_delete_file ... ok
test server::tests::test_delete_file_without_token_in_config ... ok
test server::tests::test_index ... ok
test server::tests::test_index_with_landing_page ... ok
test server::tests::test_index_with_landing_page_file ... ok
test server::tests::test_index_with_landing_page_file_not_found ... ok
test server::tests::test_list ... ok
test server::tests::test_list_expired ... ok
test server::tests::test_payload_limit ... ok
test server::tests::test_upload_duplicate_file ... ok
test server::tests::test_upload_expiring_file ... ok
test server::tests::test_upload_file ... ok
test server::tests::test_upload_file_override_filename ... ok
test server::tests::test_upload_oneshot ... ok
test server::tests::test_upload_oneshot_url ... ok
test server::tests::test_upload_remote_file ... FAILED
test server::tests::test_upload_url ... ok
test server::tests::test_version ... ok
test server::tests::test_version_without_auth ... ok
test server::tests::test_version_without_config ... ok
test util::tests::test_get_expired_files ... ok
test util::tests::test_glob_match ... ok
test util::tests::test_sha256sum ... ok
test util::tests::test_system_time ... ok

failures:

---- paste::tests::test_paste_data stdout ----
thread 'paste::tests::test_paste_data' panicked at src/paste.rs:489:9:
assertion `left == right` failed
  left: "8c712905b799905357b8202d0cb7a244cefeeccf7aa5eb79896645ac50158ffa"
 right: "70ff72a2f7651b5fae3aa9834e03d2a2233c52036610562f7fa04e089e8198ed"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- server::tests::test_upload_remote_file stdout ----
thread 'server::tests::test_upload_remote_file' panicked at src/server.rs:1061:9:
assertion `left == right` failed
  left: "8c712905b799905357b8202d0cb7a244cefeeccf7aa5eb79896645ac50158ffa"
 right: "70ff72a2f7651b5fae3aa9834e03d2a2233c52036610562f7fa04e089e8198ed"


failures:
    paste::tests::test_paste_data
    server::tests::test_upload_remote_file

test result: FAILED. 34 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.24s

error: test failed, to rerun pass `--lib`
@tessus
Copy link
Collaborator Author

tessus commented Mar 6, 2024

The file https://upload.wikimedia.org/wikipedia/en/a/a9/Example.jpg has changed and thus the sha256 is wrong. I am wondering if we can find a file that doesn't change - ever. Hmm, maybe something in a repo.

Unfortunately the rustypaste logo is 224,372 bytes while the Example jpg is currently 25,503 bytes.

I'll think of something.

@tessus
Copy link
Collaborator Author

tessus commented Mar 6, 2024

Ok, so all your PNGs in your repos are around 200-250k. Wikipedia does not have permanent links, but they archived the previous one and this link should never change.
I'll update the link, so the hash stays the same.

@orhun
Copy link
Owner

orhun commented Mar 6, 2024

Sounds good to me!

@tessus
Copy link
Collaborator Author

tessus commented Mar 6, 2024

Small problem. for some reason the hash even changed for the old file (different link, but same file). I suspect that the sha256_digest function in util.rs takes the filename into account. Weird, I don't think this should behave that way.

@orhun
Copy link
Owner

orhun commented Mar 6, 2024

I'm thinking if we should just remove those tests altogether or gate them behind a feature flag. They have been failing spuriously for some time.

@tessus
Copy link
Collaborator Author

tessus commented Mar 6, 2024

Which tests? all of the ones in paste.rs? That would be rather brutal.

Btw, I have the fix for the same filename ready. Just waiting for the other PR to be merged, because the file op would then require an .await.

One more thing: it seems that the auto-merge for the depandabot PRs doesn't work. Neither here, nor in the cli repo.

@orhun
Copy link
Owner

orhun commented Mar 6, 2024

Which tests? all of the ones in paste.rs? That would be rather brutal.

I meant only the ones that require fetching stuff from a remote server (i.e. Wikipedia) - but I guess they are fine.

One more thing: it seems that the auto-merge for the depandabot PRs doesn't work. Neither here, nor in the cli repo.

Do you think we should have Mergify? What do you think of auto-merging PRs?

@tessus
Copy link
Collaborator Author

tessus commented Mar 6, 2024

I meant only the ones that require fetching stuff from a remote server (i.e. Wikipedia) - but I guess they are fine.

Ah, ok. Yep, they should be fine now. Well, it's always a risk, e.g. if there's a connection issue from the runner, which happens now and then.
But how would you test a remote fetch? We could setup a local web service that fetches a local file. But that might be an overkill.

Do you think we should have Mergify?

It's setup in rustypaste-cli, but doesn't seem to work.

What do you think of auto-merging PRs?

Complex topic. It entails a certain risk, although in can be minimized. If the automerge only happens, if all tests pass successfully, then it should be fine. If there's also a way to limit it to minor and patch upgrades, it's even better.
The problem is that you have a lot of projects and many of them have dependencies. Manually managing all of them is a lot of work and you can't test all of them yourself.
Thus I am afraid, even if auto-merging is not the 100% most perfect solution, it might be the only viable one.

P.S.: I am only talking about dependabot PRs.

@orhun
Copy link
Owner

orhun commented Mar 6, 2024

Yeah, fair point. I think I will set up auto merging again soon. Can you shoot me an issue so that I don't forget? 🙏🏼

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 a pull request may close this issue.

2 participants