Skip to content

Conversation

@dbettini
Copy link
Collaborator

  • Removed the conflation of integration and API tests
  • Added infrastructure and entry points integration subsections
  • Added a note to API tests to mention the distinction from integraiton

- Removed the conflation of integration and API tests
- Added infrastructure and entry points integration subsections
- Added a note to API tests to mention the distinction from integraiton
@dbettini dbettini force-pushed the integration-api-test-updates branch from 3b744cc to 8a9e01e Compare September 16, 2024 12:41
Running the tests on test infrastructure should be preferred to mocking, unlike in unit tests. Ideally, a full application instance would be run, to mimic real application behavior as close as possible.
This usually includes running the application connected to a test database, inserting fake data into it during the test setup, and doing assertions on the current state of the database. This also means integration test code should have full access to the test infrastructure for querying.
> [!NOTE]
> Regardless of whether using raw queries or the ORM, simple queries should be used to avoid introducing business logic within tests.
Copy link
Member

Choose a reason for hiding this comment

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

@dbettini I don't understand what's meant by this. Can you elaborate a bit, please (in reply to this comment)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm guessing you're asking about the note about simple queries? If yes, I'm talking about using simple SELECTs with WHERE, instead of using more complex custom queries, since that could result in the test giving a false positive/negative. E.g. query being too specific, making it impossible to realize that something else exists in the database which would fail the test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understood it as "don't repeat business logic in tests". So using simple queries to check if something is changed (or unchanged) is preferred over repeating a business model query and asserting the output is correct.

Copy link
Member

Choose a reason for hiding this comment

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

@dbettini, do you mean queries used to perform assertions at the end of the test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@MiroDojkic queries used to fetch the data that will be asserted on

Comment on lines 100 to 106
#### Entry points

Integration test entry points can vary depending on the application use cases. These include services, controllers, or the API. These are not set in stone and should be taken into account when making a decision. For example:
- A use case that can be invoked through multiple different protocols can be tested separately from them, to avoid duplication. A tradeoff in this case is the need to write some basic tests for each of the protocols.
- A use case that will always be invokeable through a single protocol might benefit enough from only being tested using that protocol. E.g. a HTTP API route test might eliminate the need for a lower level, controller/service level test. This would also enable the testing of authorization within the same test, which might not have been possible otherwise depending on the technology used.

Multiple approaches can be used within the same application depending on the requirements, to provide sufficient coverage.
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to define a "use-case" to ensure different readers don't come up with different understandings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm, to be honest none comes to mind that wouldn't fit here. Do you have anything specific in mind?

Copy link
Member

Choose a reason for hiding this comment

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

I think for a junior, "use-case" can mean method invocation, while in software architecture, use-case usually relates to business logic use-case encapsulated in an app service method, command or query (in CQRS).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Never heard of it being used that way, maybe because it was explained as a business logic use case even in college in my case, which might not be the case for everyone.
Do you think rewording "application use cases" to "business logic use cases" is enough, or you have something else in mind?

Copy link
Member

@kronicker kronicker Oct 29, 2024

Choose a reason for hiding this comment

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

@MiroDojkic @dbettini Would it make sense to add a glossary sections to these tech guides so we can make terminology clear to a less experience reader while keeping the text itself more concise?
Edit: Just realized there actually is Glossary section 🤡 so maybe we could just add this clarification to it. Next point about backlinks remains.
For bonus points, we could do backlinks to glossary throughout text for terms that could not be super obvious or are domain specific.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added to glossary, backlinks seemed like overkill for now


Integration test entry points can vary depending on the application use cases. These include services, controllers, or the API. These are not set in stone and should be taken into account when making a decision. For example:
- A use case that can be invoked through multiple different protocols can be tested separately from them, to avoid duplication. A tradeoff in this case is the need to write some basic tests for each of the protocols.
- A use case that will always be invokeable through a single protocol might benefit enough from only being tested using that protocol. E.g. a HTTP API route test might eliminate the need for a lower level, controller/service level test. This would also enable the testing of authorization within the same test, which might not have been possible otherwise depending on the technology used.
Copy link
Member

Choose a reason for hiding this comment

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

Authorization should be tested separately to increase determinism (if authorization fails, we know it failed immediately).
I assume what you meant here is that this way, we test the integration of the auth layer with other app layers?

Copy link
Collaborator Author

@dbettini dbettini Sep 17, 2024

Choose a reason for hiding this comment

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

Yes, was referring to being able to expect a 403 if trying to access an admin route with a non-admin token, for example. Maybe replace

This would also enable the testing of authorization within the same test, which might not have been possible otherwise depending on the technology used.

with

This would also enable testing the auth layer integration within these tests, which might not have been possible otherwise depending on the technology used.

Or something else?

Copy link
Member

Choose a reason for hiding this comment

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

@dbettini That makes more sense to me. Otherwise it might be interpreted as "auth is implicitly tested through other integration tests" so making it clear that this covers testing auth integration makes sense.

referring to being able to expect a 403 if trying to access an admin route

Regarding ⬆️ , I guess there are 2 test-cases I see here:

  1. Testing is our auth layer working as expected i.e. if for given auth permissions appropriate access is granted
  2. Are permissions set properly on specific route/entry point
    I think the first should be tested generically in a auth-specific tests while the second we can get implicitly from integration test on that specific entry point. While I see value in testing appropriate permissioning on a entry point, I'm wondering is it worth-while to add (all) auth permutation assertions to each entry point as, depending on the complexity of auth layer and permissioning implemented, it could blow-up number of test-cases.
    What do you think? (I might be looking too much into a simple example 😄 )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kronicker unfortunately, I don't have a catch-all answer there 😄 I do like to test the basic cases, e.g. a route only being accessible to admins, for two reasons:

  1. Other than implicitly testing the behavior of the auth layer, it also tests that the correct rules were applied. This is useful because even if our auth layer works correctly, it still doesn't mean it was correctly configured for this specific route.
  2. The level of danger in the case of an error - I feel much safer if a route that e.g. allows destructive actions for admins has a test that ensures other roles would get an error

But like you said, we should make sure to avoid combinatorial explosions, so it needs to be applied on a case by case basis IMO - this is why I didn't go into too much detail in this section, but went deeper in the When not to use section.


#### When **not** to use
- For testing of specific function logic. We should use unit tests for those.
- For extensive testing of business logic permutations beyond fundamental scenarios. Integration tests contain more overhead to write compared to unit tests and can easily lead to a combinatorial explosion. Instead, unit tests should be used for thorough coverage of these permutations.
Copy link
Member

Choose a reason for hiding this comment

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

What would combinatorial explosion mean, and how would it look like?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is the fast growth in the number of combinations that need to be tested when multiple business rules are involved. At least that's the term I've seen sometimes used in the context of writing tests 😄 It's especially important in integration tests since they have a higher chance of invoking multiple different business rules at the same time. Didn't explain the term because I guessed it could be googled. What do you think? Should a different term be used?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It falls in line with what was previously written that unit tests should be preferred way of testing each unit logic instead of testing multiple units and trying to add enough permutations/combinations to cover all use cases.

Copy link
Member

@kronicker kronicker Oct 29, 2024

Choose a reason for hiding this comment

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

@dbettini Another candidate for the glossary?

dbettini and others added 2 commits September 17, 2024 13:53
Co-authored-by: Miro Dojkic <mdojkic@gmail.com>
Also reworded Entry Points slightly.
@dbettini dbettini requested review from MiroDojkic and kronicker and removed request for ikovac November 25, 2024 14:22
@dbettini dbettini merged commit 0d3898e into feature/automated-testing Dec 5, 2024
@dbettini dbettini deleted the integration-api-test-updates branch December 5, 2024 13:55
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.

5 participants