orhun / rustypaste

A minimal file upload/pastebin service.
https://blog.orhun.dev/blazingly-fast-file-sharing
MIT License
757 stars 47 forks source link

2 tests fail on git master #253

Closed tessus closed 6 months ago

tessus commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

Sounds good to me!

tessus commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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? 🙏🏼