BlockDownloadingTest workaround

[csiki2]

Trying to do something with it...

[kiminuo]

Is the test flaky?

If it is, we typically add this: https://github.com/zkSNACKs/WalletWasabi/blob/bd3a6d723ae88fa642f86673dcd717503edb82e9/WalletWasabi.Tests/UnitTests/PeriodicRunnerTests.cs#L9-L10 for tests that are time sensitive.

[csiki2]

Is the test flaky?

If it is, we typically add this:

https://github.com/zkSNACKs/WalletWasabi/blob/bd3a6d723ae88fa642f86673dcd717503edb82e9/WalletWasabi.Tests/UnitTests/PeriodicRunnerTests.cs#L9-L10
for tests that are time sensitive.

I will give it a try, thanks.

[kiminuo]

The previous change should make it so that this change is unnecessary.

Suggested change

	await Task.Delay(2000);
	await Task.Delay(100);

[csiki2]

Are you sure?
I feel like we are risking to rerun 1000+ tests (due to failure) for 2 seconds.

[kiminuo]

It shouldn't take 2 seconds. There is no complex operation AFAIK.

Your PR title is called "BlockDownloadingTest fix". It's not a fix, it's just a workaround. If you want to fix, you should fix it properly - i.e. remove the Task.Delay completely and wait for the operation directly.

[csiki2]

Better now?

[csiki2]

I will check tomorrow whether it works without delay.

[turbolay]

I ran the 3 tests in parallel - so similar to master - 10.000 times, and didn't have any error.

I also never saw CI failing because of this. So I'll remove the extra delay and merge, feel free to reopen if someone notices an issue

[turbolay]

This PR was such a mess......... All of this to modify a test that is not failing!!
You messed up 2 different tests, also you blocked maintainers from contributing to your fork? Why??
I'm closing this PR and will merge mine, as I can't push here.

[csiki2]

There were too many misunderstanding due to adding code to a pull request without any question.
Too many people has the right to add code to a pull request and there are clearly no guidelines on when it is ok and when it is not.
Saying to a one line PR to be a mess is quite overexaggerating.

[kiminuo]

There were too many misunderstanding due to adding code to a pull request without any question.
Too many people has the right to add code to a pull request and there are clearly no guidelines on when it is ok and when it is not.
Saying to a one line PR to be a mess is quite overexaggerating.

Honestly, your attitude is fascinating to me. My response would be "Sorry, I'll do better next time.". But perhaps I'm just too old in this modern world.

[turbolay]

Saying to a one line PR to be a mess is quite overexaggerating.

Well let's see:

• The title is incorrect, as you said you are modifying BlockDownloadTest when it was BlockDownloadServiceTest.
• There is no explanation about why this modification. Is the test failing? Where did you see that? I had to test it myself all of this to see that.... the test was not failing. I also went to check CI on some of your PR and didn't see any failed run. So I had to try to write a loop, wait for 10k repetitions, but nope, not failing.
• It was not a 1 line but a 2 lines PR, and yet you've added each of these lines to a different file, in fact to the point that I was not even sure what test you wanted to modify
• I tried to fix that but I couldn't push here, I thought it was a problem on my side I've checked what was wrong, if I was connected to the wrong GitHub account etc etc... All to finally understand that you are the only contributor that blocks pushes on your PR.

Took me more or less 40 minutes to finally merge it. A lot for a 2 lines PR that doesn't fix any problem. Hence, 2 lines PR or not, it was a mess.