Try to fix reset-project Playwright tests #789
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I'm deploying the |
UI unit Tests11 tests 11 ✅ 0s ⏱️ Results for commit 6c2b514. ♻️ This comment has been updated with latest results. |
|
Can't deploy the |
rmunn
force-pushed
the
chore/try-to-fix-playwright-reset-tests
branch
from
f6165f7 to
d3e550d
Compare
|
Was able to find proof that running I'm going to remove the |
yeah that's what I said here: #765 (comment) |
Yep. I thought about that last week but wanted to verify whether changes to the C# code were enough. Proved that they weren't. However, I've also managed to prove that running BTW, I ran the projectReset.test.ts tests about 30 times, using |
|
Commit 5cc036e contains the change I had made locally, but not yet pushed, when I ran the reset-project E2E test 30 times without a failure. (It only works because of commit c87ec67, which makes the I think I'm going to push one more commit, adding a new command to the command runner called "refreshdircache". It will not call any hg commands, so its entire purpose is to run that |
|
Marking as draft PR for now because there are more changes I plan for tomorrow, so there's not much point in reviewing it in its current state. |
rmunn
force-pushed
the
chore/try-to-fix-playwright-reset-tests
branch
from
9691a40 to
87582d0
Compare
Pull in changes to deleting draft projects, necessary in order to run Playwright E2E tests more than once
If FinishReset gets an upload of an *empty* Mercurial repo, then WaitForRepoEmptyState could wait forever. To catch that situation 99% of the time, we can simply check for the existence of `00changelog.i` in the Mercurial repo store: that file is guaranteed to exist if the repo has commits, so its absence means an empty repo. To ensure belt-and-suspenders, we still need to implement a timeout in the WaitForRepoEmptyState method, but this is a good start.
We now choose a random filename and check its existence before creating it. In theory there's still a chance that another process could create the file before we get a chance to, but in practice that's never going to happen. We also create a directory rather than a file because on the very, very slim chance that another process creates a file between when we check its existence and when we create it, the Directory.Create call will fail. So it's minutely safer.
|
I replaced the call to I'm cautiously optimistic, and going to take this PR off of draft status because I think I may have finally solved the problem. |
|
NOTE: I've marked this as ready for review, so I don't want to push experimental commits while others might be reviewing this. Therefore, I've created the |
frontend/src/routes/(authenticated)/project/[project_code]/ResetProjectModal.svelte
Outdated
Show resolved
Hide resolved
This allows us to uncomment the tests we were skipping earlier, because now we can mock HttpClient and not allow HgService to hit the network during unit tests.
Especially in WaitForRepoEmptyState, we'll want to make sure the function times out after a while so we can't end up locking up the server due to some unforeseen error.
|
@hahn-kev - Addressed all your review comments. Let me know if there's anything else that needs changing. |
I forgot that .NET code will throw OperationCanceledException if passed a token that gets canceled. We don't want WaitForRepoEmptyState to throw if its timeout is reached, we want it to return as if it was successful. So we need to catch and suppress that exception. Also passed the token into Task.Delay so that if the timeout is reached during the delay, the delay will cancel immediately instead of running to completion.
Fixes merge conflict in Testing.csproj
We believe the failures in the Playwright tests are due to NFS caching the directory entry, and thus not noticing that the directory has new contents, resulting in Mercurial thinking the directory is still empty for some time after the zip file upload has completed. It is possible to force NFS to refresh its cache of a directory by doing opendir() and closedir() on that directory. I don't know if C#'s Directory.GetFiles method, which uses the openat() system call rather than opendir() and closedir(), will have the same effect — but it's worth a try.
Update: The other piece of the puzzle appears to be caching on the side of the LexBox API pod's NFS client. NFS writes are cached until one of the following events occurs (scroll down to "The sync mount option" heading):
So doing an
opendir/closedir(which actually needs to happen on the hgweb pod, not the lexbox pod) is not enough; we also need to force the lexbox pod's NFS client to send its data to the NFS server. I've implemented that by doing the equivalent ofmkdir random-name; rmdir random-nameinside the Mercurial repo after the LexBox API pod touches it (e.g. a project reset or FinishReset operation), followed by a call tolson the hgweb side. So far that seems to be working; more experimentation is needed to see whether thelsis necessary, or whether themkdir/rmdiris enough. (Or perhaps it truly does need to be the C# equivalent ofcp /dev/null random-file; rm random-fileso that aclose()system call is issued).Might fix #765.