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

Test data #2620

Merged
merged 6 commits into from
Dec 24, 2024
Merged

Test data #2620

merged 6 commits into from
Dec 24, 2024

Conversation

BCerki
Copy link
Contributor

@BCerki BCerki commented Dec 18, 2024

Prep for #2222 . It's really hard to test reg2 db changes when we still have to keep the reg1 e2e tests running, and having both reg1 and reg2 stuff in the app is v confusing for testers and POs.

This PR:

  • make a copy of the existing fixture files in mock/v1
  • creates a separate load fixture command for reg1 to pull from the mock/v1 folder
  • uses this new command in the v1 test-setup endpoint. This endpoint is called to set up e2e tests (definitely) and the data in the dev env (I think, right @dleard? I see it called here: cas-registration/helm/cas-registration/templates/backend/job/load-dev-data.yaml)
  • changes Operator 1 to Alpha Industries in reg2 mock data only to confirm that separate data can coexist (card Improve reg2 mock data #2627 handling improving the reg2 data)
  • all the existing make commands refer to the original load fixture command, so no changes needed. All the model test fixtures also refer to the original directory so also no changes needed
  • happo is flake

@BCerki BCerki force-pushed the test-data branch 2 times, most recently from 961c670 to 1abc736 Compare December 19, 2024 18:57
@BCerki BCerki marked this pull request as ready for review December 19, 2024 18:58
@@ -0,0 +1,236 @@
[
Copy link
Contributor

Choose a reason for hiding this comment

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

I notice this is in v1 directory, but facilities are only relevant to Reg 2. Is this an oopsie?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! There's a few others I can delete too, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might make reg1 e2e run faster too 👀

"bcghg_id": "23219990013",
"created_at": "2024-1-20T15:27:00.000Z",
"regulated_products": [1, 2, 3, 4],
"status": "Transferred",
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to have the "Transferred", "Closed", "Temporarily Shutdown", "Registered" statuses for v1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reg1 e2e tests do appear to be looking for those reg2 statuses, because CI fails when I take them out (and the code snippet below is maybe relevant?). I'll see if I can get @shon-button 's help figuring out where the failing assertions are, but if it's not straightforward, are you okay with me leaving these statuses in for this PR, and I'll add an AC about getting rid of them to #2627 ?

In /home/briannacerkiewicz/cas-registration/bciers/apps/registration1/e2e/poms/operations.ts:

expectedValues = [
              OperationStatus.PENDING,
              OperationStatus.APPROVED,
              OperationStatus.DECLINED,
              OperationStatus.CHANGES_REQUESTED,
              OperationStatus.REGISTERED,
              // Below values are not possible in Registration1
              OperationStatus.CLOSED,
              OperationStatus.TEMPORARILY_SHUTDOWN,
              OperationStatus.TRANSFERRED,

bc_obps/Makefile Outdated
@@ -92,6 +92,11 @@ superuser: ## create a superuser
superuser:
$(POETRY_RUN) python $(MANAGE_PY) create_superuser

loadfixtures_v1: ## add all registration fixture files to the db
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused by the naming conventions - is loadfixtures_v1 for Reg1 and loadfixtures for Reg2?

Copy link
Contributor Author

@BCerki BCerki Dec 19, 2024

Choose a reason for hiding this comment

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

Yes, I'll make a comment and add some docs. This is to match how we've done the nx apps and the endpoints (so when we delete reg1 we're just left with reg instead of reg2, v2, etc.)

@BCerki BCerki force-pushed the test-data branch 3 times, most recently from 82826d7 to 7a65cc5 Compare December 19, 2024 23:47
Copy link
Contributor

@andrea-williams andrea-williams left a comment

Choose a reason for hiding this comment

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

Approving. Thanks for doing this Bri - it will help out a lot of people! :D

@Sepehr-Sobhani Sepehr-Sobhani merged commit 0e93aea into develop Dec 24, 2024
42 checks passed
@Sepehr-Sobhani Sepehr-Sobhani deleted the test-data branch December 24, 2024 18:08
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.

3 participants