Use tempfile for temporary e2e test directories

[spacebear21]

tempfile::tempdir() ensures that each test operates in an independent directory, preventing contention between concurrent tests.

Closes #859

[coveralls]

Pull Request Test Coverage Report for Build 16207791035

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 85.558%

---

Totals
Change from base Build 16204473407:	0.0%
Covered Lines:	7571
Relevant Lines:	8849

---

💛 - Coveralls

[spacebear21]

This is a draft because the e2e test is currently failing due to localcert errors I do not understand... Hoping @nothingmuch can take a look tomorrow.

Nevermind, the cert issue is being caused by the changeset in #869 which I was building on top of locally. This standalone PR may should be good to merge upon review.

[Johnosezele]

ACK f4722de. I built and ran the e2e tests concurrently. The v1 test now passes consistently, which confirms the fix for the file-locking race condition. I am aware of the v2 test failure, but understand it is an unrelated issue. I agree this can be merged.

Reproducing the Issue on master

To begin, I checked out the master branch to confirm the original problem. I ran the end-to-end tests in parallel with the following command:

cargo test --test e2e --features _danger-local-https,v1,v2 -- --test-threads=8

This successfully reproduced the issue. The tests failed with file-locking errors due to the race condition, as expected:

running 2 tests
Error: Database operation failed: IO error: could not acquire lock on "/var/folders/.../receiver_db/db": Os { code: 35, kind: WouldBlock, message: "Resource temporarily unavailable" }
...
test e2e::send_receive_payjoin_v1 ... FAILED
test e2e::send_receive_payjoin_v2 ... FAILED

failures:
    e2e::send_receive_payjoin_v1
    e2e::send_receive_payjoin_v2

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out;

Verifying the Fix on the PR Branch

Next, I checked out this pull request branch (pr-868) and ran the exact same command to verify the fix:

cargo test --test e2e --features _danger-local-https,v1,v2 -- --test-threads=8

With these changes, the v1 test now passes consistently, demonstrating that the file-locking race condition has been resolved. The output was as follows:

running 2 tests
...
test e2e::send_receive_payjoin_v1 ... ok
test e2e::send_receive_payjoin_v2 ... FAILED
...

failures:

---- e2e::send_receive_payjoin_v2 stdout ----
Database running on 127.0.0.1:55001
Directory server binding to port [::]:60819
OHTTP relay binding to port [::]:60820
2025-07-11T13:30:48.543147Z ERROR payjoin_directory: TLS accept error: received fatal alert: DecryptError
2025-07-11T13:30:48.545147Z ERROR payjoin_directory: TLS accept error: received fatal alert: DecryptError

thread 'e2e::send_receive_payjoin_v2' panicked at payjoin-cli/tests/e2e.rs:314:13:
Fallback send was not detected
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    e2e::send_receive_payjoin_v2

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out;

I noted that the send_receive_payjoin_v2 test still fails, but as you mentioned, this is due to an unrelated TLS issue stemming from changes in PR #869.

[nothingmuch]

utACK, my suggestion can be rejected or done in followup, no strong opinion

editing the test is pretty easy so i don't think it's super important, but it's nice to not have to remember how to do it with the tempdir api

[nothingmuch]

The docs say:

Note that if the program exits before the TempDir destructor is run, such as via std::process::exit(), by segfaulting, or by receiving a signal like SIGINT, then the temporary directory will not be deleted.

i would bet cargo test SIGINT has a handler that does something smarter, do you know if this is the case?

For debugging we should probably also have something like this:

Suggested change

	let temp_dir = tempdir()?;
	// FIXME parse the var as truthy don't just check is_ok()
	let disable_cleanup = env::var("KEEP_TEMPDIR").is_ok();
	let temp_dir = Builder::new().disable_cleanup(disable_cleanup).tempdir()?;

[spacebear21]

I'm not aware of any special SIGINT handling by cargo test, but IMO not deleting temp files on sigint is no big deal, since each tempdir is unique it's unlikely there would ever be a conflict from a previous interrupted test run.

[zealsham]

ACK , this works well and is a better solution than what i came up with before losing my files . I appened uuid to the temp_dir for each test to make it unique .