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

Create E2E tests for reset-project feature #550

Merged
merged 21 commits into from
Feb 14, 2024
Merged

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Feb 6, 2024

Fix #440.
Fix #515.

This PR creates two tests:

  • Playwright E2E test that tests the function of the reset project modal, ensuring that it can reset a project to empty, and restore it to a previous state from a downloaded .zip file.
  • C# integration test that verifies that Send/Receive still works after a project has been reset to empty. (We don't test Send/Receive after a project reset has uploaded a .zip file, because the Playwright test already checks that the uploaded .zip file can bring the project repo back to the exact same state it was in before.)

Copy link

github-actions bot commented Feb 6, 2024

UI unit Tests

1 tests  ±0   1 ✅ ±0   0s ⏱️ ±0s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 81f4e30. ± Comparison against base commit 02a32cd.

♻️ This comment has been updated with latest results.

@rmunn rmunn linked an issue Feb 6, 2024 that may be closed by this pull request
@rmunn
Copy link
Contributor Author

rmunn commented Feb 9, 2024

After discussion with Kevin today, we decided to lower the refreshinterval in hgweb.hgrc from 20 seconds (default value) to 5 seconds. That means that hgweb will run a directory listing every 5 seconds to find any new projects that have been created since the last time it was run. Without this, the unit tests were sometimes running into an issue where they would create a new project, then try to push commits to it only for hgweb to say "No such repo exists". We've added a 5-second delay in the affected tests, so that should be guaranteed to be pushing to an hgweb instance that knows about the newly-created project.

@rmunn rmunn force-pushed the chore/test-reset-repo branch from 7899354 to ec35883 Compare February 9, 2024 05:01
@rmunn
Copy link
Contributor Author

rmunn commented Feb 9, 2024

Rebased on top of develop; now the number of commits is actually correct.

@rmunn rmunn marked this pull request as ready for review February 9, 2024 05:02
Copy link

github-actions bot commented Feb 9, 2024

C# Unit Tests

39 tests  ±0   39 ✅ ±0   5s ⏱️ ±0s
 7 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 81f4e30. ± Comparison against base commit 02a32cd.

♻️ This comment has been updated with latest results.

rmunn added 10 commits February 9, 2024 16:14
This should help minimize the number of times the Send/Receive tests
run into a race condition where a new test has been created on the
hgweb server, but Mercurial hasn't yet picked it up.
The new SendReceiveAfterProjectReset test is passing now that hgweb is
checking for new repos every 5 seconds; let's try to ensure that it
stays reliable by adding 5-second sleeps in the right places.
Now the resetProject.test.ts file will run again.
Now we check after the project has been reset to verify that it's at the
same hash, with the same number of files at the top level, as before the
reset-and-upload process.
This fixture creates a temporary project and deletes it afterwards.
This is about the smallest Mercurial repo possible that isn't empty: a
repo with just one commit of an empty file. We'll use this in E2E
testing to populate new projects.
Greatly expanded the reset-project Playwright test:

- It uses a temp project now, whose name is based on a hash of the test
  title and browser settings, so Chromium and Firefox will use different
  project names. This will allow the test to run in parallel with itself.
- It uploads a known initial state of a Mercurial repo, which is a tiny
  zip file containing just one commit.
- It checks that the project-reset modal dialog can reset the project to
  an empty state or upload from a zip file correctly.

Combined with the .NET test that confirms that a project can still be
used with Send/Receive after resetting to an empty state, that should
cover just about everything.
@rmunn rmunn force-pushed the chore/test-reset-repo branch from bde8e27 to 8a98102 Compare February 9, 2024 09:15
@rmunn
Copy link
Contributor Author

rmunn commented Feb 9, 2024

Rebased on top of latest develop to fix merge conflict in fixtures.ts.

Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, I'll run it myself on Monday. There was one case where localhost was hard-coded in.

@hahn-kev hahn-kev requested a review from myieye February 12, 2024 03:36
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @myieye would you review the playwright tests?

Copy link
Contributor

@myieye myieye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
I just have a small pile of refactoring ideas.

rmunn and others added 4 commits February 13, 2024 09:07
If two copies of the test run simultaneously in Chromium and Firefox,
you can end up with the situation where one of them sees two copies of
the test project in the admin dashboard. And while they have different
project codes, until now they had the same project name. However, since
the admin dashboard code to click on the project uses the project name
(with `{exact: true}`), the locator was seeing two copies of the
project, and therefore throwing an error.

This commit fixes that by putting the test ID (which is different for
each browser) into the project name as well as the project code, so that
the admin dashboard locators will only ever find a single project.
One locator was using this.page instead of this.componentLocator,
because of an error that turned out to be unrelated. So it can be
switched back to using this.componentLocator again.
This should save some time, as the browser tab won't have to load and
render the hgweb HTML. It also lets us make more fine-grained checks of
the results, since we're getting back JSON structures that we know will
have certain properties.
Note that this doesn't actually improve the type safety of this code at
all, since Typescript just takes our word for it that we know the
structure of the JSON returned by that particular fetch request. But it
makes the linter happy not to get Typescript errors.
We're changing the clickResetProject() method to actually return a
ResetProjectModal instance as the return type suggests, and to wait for
it so there's no need for the `waitFor()` calls in the calling function.
@rmunn
Copy link
Contributor Author

rmunn commented Feb 14, 2024

All conversations resolved, all checks passing, so I'm going to merge this in.

A couple of conversations were resolved a little early, IMHO, so I created #569 and #570 to track the work remaining to do after I merge this PR.

@rmunn rmunn merged commit 157b420 into develop Feb 14, 2024
13 checks passed
@rmunn rmunn self-assigned this Feb 23, 2024
@hahn-kev hahn-kev deleted the chore/test-reset-repo branch July 8, 2024 06:57
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 this pull request may close these issues.

Resumable issue pushing a new project Create test for project reset
3 participants