Skip to content

polish(auth): Better readme for testing commands #18372

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.

Sign up for GitHub

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

Merged
dschom merged 1 commit into from
Feb 14, 2025
Merged

polish(auth): Better readme for testing commands #18372

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 26 additions & 20 deletions packages/fxa-auth-server/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,36 +32,42 @@ Linting will also be run for staged files automatically via Husky when you attem

## Testing

Run tests with:
Before running tests make sure the correct services are running. If auth server is running in pm2 weird behavior can ensue, so
Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau Feb 13, 2025

Choose a reason for hiding this comment

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

Suggested change
Before running tests make sure the correct services are running. If auth server is running in pm2 weird behavior can ensue, so
If auth server is running in pm2 weird behavior can ensue, so

Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau Feb 13, 2025

Choose a reason for hiding this comment

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

For unit-tests, we actually want none of the services to be running?

Copy link
Copy Markdown
Contributor Author

@dschom dschom Feb 13, 2025

Choose a reason for hiding this comment

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

Unit tests do not need anything to be running. This is one of the main things that separates them from integration tests. If you look at our CI, unit tests can execute in total isolation. Now, should nothing be running at all? TBD… but it probably shouldn’t matter.

However, there's also this pre-existing comment in the readme that said “Note: stop the auth-server before running tests. Otherwise, they will fail with obscure errors.” This was written before we made a distinction between unit and integration tests... So not sure if it matters for unit-tests. I'd have to really vet this to be sure. I know for a fact a running auth-server messes with our test/remote tests.

Aside from quirks with auth server testing, in general, if you are working on unit tests I think it's good to not have ancillary things running. Do so and you might just be writing an integration test and not realizing it. That's why I think it's good practice to just run them after doing a yarn stop... ymmv.

prior to running tests we recommend creating a clean slate by running `yarn stop`.

npm test
Now we can start testing. To run unit tests:

Run specific tests with the following commands:
- `nx test-unit fxa-auth-server`
_Note this matches how auth server unit tests jobs in CI._

```bash
# Test only test/local/account_routes.js
# Note: This command does not work for remote tests.
npm test -- test/local/account_routes.js
To run integration tests, we have to make sure databases and auxillary services required for integration testing are spun up.
So run:

# Grep for "SQSReceiver"
NODE_ENV=dev npx mocha -r esbuild-register test/*/** -g "SQSReceiver"
```
- `yarn start infrastructure`
- `nx start fxa-customs-server`
Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau Feb 13, 2025

Choose a reason for hiding this comment

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

Maybe this would be a good spot to explain why customs is needed?

Copy link
Copy Markdown
Contributor Author

@dschom dschom Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I thought about this, but people can look at the code. And the reason may change overtime. Customs is an auxiliary service. I actually feel like customs deserves its own auth server specific section.


To select a specific glob of tests to run:
Now unit tests can be executed:

```
npm test -- test/local/account_routes.js test/local/password_*
```
- `nx test-integration fxa-auth-server`
_Note this matches how auth server unit tests jobs in CI._

To run a certain suite of tests (e.g. all remote tests):
For general development based testing, specific tests can be targeted using the test script or use mocha directly:

```
npm test -- test/remote
```
_From packages/fxa-auth-server:_

- `yarn test -- test/local/account_routes.js`
- `yarn test -- test/local/account* test/local/password_*`
- `yarn test -- test/remote`
- `NODE_ENV=dev npx mocha -r esbuild-register test/*/** -g "SQSReceiver"`

Notes / Tips:

- Note: stop the auth-server before running tests. Otherwise, they will fail with obscure errors.
- You can use `LOG_LEVEL`, such as `LOG_LEVEL=debug` to specify the test logging level.
- For quick enviroment config, consider running tests with a .env file and the dotenv command. For example: `dotenv -- yarn workspace fxa-auth-server:test-integration remote`
Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau Feb 13, 2025

Choose a reason for hiding this comment

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

Should we note more specifically which variables are required?

Copy link
Copy Markdown
Contributor

@LZoog LZoog Feb 13, 2025

Choose a reason for hiding this comment

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

He does mention below: recovery-phone tests require twilio testing credentials!

I think those might be the only ones?

- you can use `LOG_LEVEL`, such as `LOG_LEVEL=debug` to specify the test logging level.
- recovery-phone tests require twilio testing credentials!
- recovery-phone-customs tests require that customs server is running. So run `nx start fxa-customs-server` prior to executing tests.

_Other Stuff_
This package uses [Mocha](https://mochajs.org/) to test its code. By default `npm test` will run a series of NPM test scripts and then lint the code:

Refer to Mocha's [CLI documentation](https://mochajs.org/#command-line-usage) for more advanced test configuration.
Expand Down