chore: Update integration test project setup instructions. #877

jonathanedey · 2023-11-13T17:38:06Z

Updated the integration test project setup instructions to be more accurate to the current setup flow.

egilmorez

Thanks for sending!

Here are some style things to look at. I think some of them apply to your other, similar doc PRs, so I'll wait on those to see if you can/want to apply the same kinds of fixes in them.

egilmorez · 2023-11-14T17:10:23Z

CONTRIBUTING.md

+Integration tests are executed against a real life Firebase project. If you do not already
+have one suitable for running the tests against, you can create a new project in the
+[Firebase Console](https://console.firebase.google.com) following the setup guide below.
+Otherwise you can obtain the following credentials from your current project:


"Otherwise" caught me off guard for a moment.

How about, "If you already have a Firebase project, you'll need to obtain credentials for <whatever they're for>:"

And yeah, what are they for? I'm not 100% clear on how they relate to the various product area setup procedures detailed below. I suspect it has to do with service accounts and Auth, but spelling it out up front could help.

Good point. Something like:

If you already have a Firebase project, you'll need to obtain credentials to communicate and authorize access to your Firebase project.

Do you suggest adding the specific purpose in the sub points below? Something like:

Service account certificate: This allows access to your Firebase project through a service account which is required for all integration tests. This can be downloaded...

Web API key: This allows for Auth sign-in needed for some Authentication and Tenant Management integration tests. This is displayed...

That's a really good thought! It's good to be brief, but when it comes to credentials or anything security-related, people like to know what's going on and why.

Great, added those in!

egilmorez · 2023-11-14T17:11:03Z

CONTRIBUTING.md

+
+
+1. Service account certificate: This can be downloaded as a JSON file from the 
+**'Settings > Service Accounts'** tab of the Firebase console when you click the


Are the single quotes needed here and below? I think just boldface is what we want for UI labels.

I don't have any strong opinions here so we can go with that. To clarify is this for the "UI > UI > UI" labels only or for all the UI like buttons?

I'm thinking it's for everything; if the ** characters give us boldface, then we don't need the ' characters.

CONTRIBUTING.md

jonathanedey · 2023-11-14T21:42:20Z

CONTRIBUTING.md

+3. Enable Realtime Database:
+   1. Go to the Firebase Console, and select **Realtime Database** from the **Build** menu.
+   2. Click on the **Create Database** button. You can choose to set up the Realtime Database
+   either in the locked mode or in the test mode.
+   3. In the **Data** tab click on the kebab menu (3 dots) and select **Create Database**.
+   4. Enter your Project ID (Found in the **General** tab in **Account Settings**) as the
+   **Realtime Database reference**. Again, you can choose to set up the Realtime Database
+   either in the locked mode or in the test mode.


I do what to highlight that this portion is asking to create 2 separate Realtime Databases.

The first being the default one which has a different naming convention than what is used in our integration tests.

The second being named using the project id of the firebase project which is the naming convention in our integration tests.

The reason for this is users are not allowed to enter a custom name for their first rtdb and changing the integration test references will break current test projects. This will be confusing so I wondering if I should go into detail here as to why.

Definitely -- I didn't quite gather that from reading the steps (it's possible I was confused because we talk about the a database "ref" value for the targeted part of a path in CF3, but that's not really relevant here).

Yup, I would definitely add some text to clarify that.

egilmorez

Good thought on clarifying the RTDB situation. After you push that, I should be able to approve, thanks!

egilmorez · 2023-11-14T23:11:29Z

CONTRIBUTING.md

+3. Enable Realtime Database:
+   1. Go to the Firebase Console, and select **Realtime Database** from the **Build** menu.
+   2. Click on the **Create Database** button. You can choose to set up the Realtime Database
+   either in the locked mode or in the test mode.
+   3. In the **Data** tab click on the kebab menu (3 dots) and select **Create Database**.
+   4. Enter your Project ID (Found in the **General** tab in **Account Settings**) as the
+   **Realtime Database reference**. Again, you can choose to set up the Realtime Database
+   either in the locked mode or in the test mode.


Definitely -- I didn't quite gather that from reading the steps (it's possible I was confused because we talk about the a database "ref" value for the targeted part of a path in CF3, but that's not really relevant here).

Yup, I would definitely add some text to clarify that.

egilmorez · 2023-11-15T19:12:46Z

CONTRIBUTING.md

@@ -168,6 +168,11 @@ Set up your Firebase project as follows:
   1. Go to the Firebase Console, and select **Realtime Database** from the **Build** menu.
   2. Click on the **Create Database** button. You can choose to set up the Realtime Database
   either in the locked mode or in the test mode.
+
+   > **Note:** Integration tests are not ran against the default Realtime Database reference and are


This note works great but needs some nitting from a TW: use "run" instead of "ran", and check the spelling of reference in line 174.

With those nits, LGTM.

egilmorez

With my last nit, LG!

jonathanedey · 2023-11-15T20:01:08Z

Thank you for the review @egilmorez! I'll mirror these changes in the other repos and get them back to you.

lahirumaramba · 2023-12-18T16:48:49Z

CONTRIBUTING.md

+which is required for all integration tests. This can be downloaded as a JSON file from the 
+**Settings > Service Accounts** tab of the Firebase console when you click the
+**Generate new private key** button. Copy the file into the repo so it's available at
+`integration_cert.json`.


Let's add a line on properly managing service accounts here as we will discourage this practice in the future.
(Service accounts should be carefully managed, Don't submit service account keys to source code repositories)

Sounds good, added a note under that step. Let me know if that works!

jonathanedey changed the title ~~Update integration test project setup instructions.~~ chore: Update integration test project setup instructions. Nov 13, 2023

jonathanedey requested review from lahirumaramba and egilmorez November 14, 2023 15:13

egilmorez reviewed Nov 14, 2023

View reviewed changes

jonathanedey commented Nov 14, 2023

View reviewed changes

egilmorez reviewed Nov 14, 2023

View reviewed changes

egilmorez reviewed Nov 15, 2023

View reviewed changes

egilmorez approved these changes Nov 15, 2023

View reviewed changes

jonathanedey assigned lahirumaramba Nov 22, 2023

jonathanedey marked this pull request as ready for review November 30, 2023 22:44

lahirumaramba approved these changes Dec 18, 2023

View reviewed changes

lahirumaramba reviewed Dec 18, 2023

View reviewed changes

jonathanedey added 8 commits December 18, 2023 17:48

Update integration test project setup instructions.

7e75129

fix: remove duplicate

e7e6467

fix: addresses some of the code review notes

dbecd2d

fix: addresses remaining code review notes and some typos

94d4720

Add note on 2nd rtdb reference.

e40accb

fix: nits

e62e6ef

fix: pencil

ab755c2

Added service account management note.

ed4029c

jonathanedey force-pushed the je-project-setup branch from 7b4a083 to ed4029c Compare December 18, 2023 22:48

jonathanedey merged commit 2f111fb into master Dec 19, 2023
6 checks passed

jonathanedey deleted the je-project-setup branch December 19, 2023 17:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Update integration test project setup instructions. #877

chore: Update integration test project setup instructions. #877

jonathanedey commented Nov 13, 2023

egilmorez left a comment

egilmorez Nov 14, 2023

jonathanedey Nov 14, 2023

egilmorez Nov 14, 2023

jonathanedey Nov 14, 2023

egilmorez Nov 14, 2023

jonathanedey Nov 14, 2023

egilmorez Nov 14, 2023

jonathanedey Nov 14, 2023

jonathanedey Nov 14, 2023

egilmorez Nov 14, 2023

egilmorez left a comment

egilmorez Nov 14, 2023

egilmorez Nov 15, 2023

jonathanedey Nov 15, 2023

egilmorez left a comment

jonathanedey commented Nov 15, 2023

lahirumaramba Dec 18, 2023

jonathanedey Dec 18, 2023



		1. Service account certificate: This can be downloaded as a JSON file from the
		'Settings > Service Accounts' tab of the Firebase console when you click the

chore: Update integration test project setup instructions. #877

chore: Update integration test project setup instructions. #877

Conversation

jonathanedey commented Nov 13, 2023

egilmorez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egilmorez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egilmorez left a comment

Choose a reason for hiding this comment

jonathanedey commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment