Add BDD tests for Token Feature to RSTUF API #369

kairoaraujo · 2023-06-20T08:18:19Z

The current BDD Tokens tests are part of integration tests in the RSTUF umbrella repository.
This BDD feature test doesn't belong to the integration tests once API Token authentication/authorization (auth) feature is a single component.

The BDD test is still valid once it describes very well the expected behavior.

This commit adds the test to the API and is part of the repository-service-tuf/repository-service-tuf#41

Closes #281

codecov · 2023-06-20T08:21:56Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (c82c49d) 96.30% compared to head (3dc4390) 96.30%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #369   +/-   ##
=======================================
  Coverage   96.30%   96.30%           
=======================================
  Files          19       19           
  Lines         785      785           
=======================================
  Hits          756      756           
  Misses         29       29

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

The functional tests for token is not cross-component and it removes from the umbrella repository. The tests are added to the RSTUF API (PR repository-service-tuf/repository-service-tuf-api#369) This is part of repository-service-tuf#41 Closes: repository-service-tuf#267 Signed-off-by: Kairo de Araujo <[email protected]>

kairoaraujo · 2023-06-22T01:04:10Z

@KAUTH its also related to repository-service-tuf/repository-service-tuf#364

* Remove token functional tests The functional tests for token is not cross-component and it removes from the umbrella repository. The tests are added to the RSTUF API (PR repository-service-tuf/repository-service-tuf-api#369) This is part of #41 Closes: #267 Signed-off-by: Kairo de Araujo <[email protected]> * Remove auth from FT Removes the authentication/authorization as part of the Gherkin for the features as it is not a requirement and depends on the setup. The built-in authentication is a explicity feature in the RSTUF API. It simplifies the BDD/Functional Tests for RSTUF It is part of #41 Closes #269 Signed-off-by: Kairo de Araujo <[email protected]> * Remove auth from RSTUF deployment in the FT Remove auth as enabled from the RSTUF API Signed-off-by: Kairo de Araujo <[email protected]> --------- Signed-off-by: Kairo de Araujo <[email protected]>

tests/bdd/features/tokens/generate.feature

tests/bdd/features/tokens/login.feature

tests/bdd/tokens/test_generate.py

MVrachev · 2023-06-26T12:38:00Z

tests/bdd/tokens/test_generate.py

+    target_fixture="response",
+)
+def the_admin_sends_with_invalid_expires_in_payload(
+    test_client, headers, payload


Those will be calculated with the new values of the new Examples table, correct?

@kairoaraujo am I correct here?

KAUTH · 2023-06-26T17:54:44Z

tests/bdd/features/tokens/login.feature

+    Admin has deployed and bootstrapped RSTUF
+
+    Scenario Outline: Login using RSTUF API
+        Given the API requester prepares 'data' with <username>, <password>, <scope>, <expires>


Nit: instead of 'data' we could use 'token_data' as it is named in the resource or otherwise 'payload' or 'request_body' (swagger notation)

KAUTH · 2023-06-26T18:17:59Z

tests/bdd/tokens/test_generate.py

+    """Admin uses HTTP API to generate a token."""
+
+
+@given("the admin has the admin password", target_fixture="admin_passwd")


This is not really used anywhere. We can either keep it to make clear that it's an admin user and add a pass or we might as well remove it since we specify that "Admin uses ..., so it's safe to assume they have the admin password.

KAUTH · 2023-06-26T18:29:21Z

tests/bdd/tokens/test_generate.py

+    "'write:token' scope",
+    target_fixture="access_token",
+)
+def the_admin_has_generated_an_access_token_with_write_token(access_token):


Instead of using the access_token fixture that has a predetermined set of scopes in the scope key of data, we could create a function or fixture that takes the scope as an argument. This way we know that the test is passing because it has that specific required scope (in this case only write:token) and not because of some other one by accident.

tests/bdd/tokens/test_generate.py

KAUTH · 2023-06-26T18:46:39Z

tests/bdd/features/tokens/login.feature

Add an invalid expire in payload scenario for the login as we do for the generate in

repository-service-tuf-api/tests/bdd/features/tokens/generate.feature

Line 31 in ddb8f74

When the admin sends a POST request to '/api/v1/token/new' with invalid 'expires' in 'payload'

I think we don't check that in our code as well.
We relly that

repository-service-tuf-api/repository_service_tuf_api/token.py

Line 212 in 3f8409c

expires = datetime.fromtimestamp(token.get("exp"))

will fail.

@kairoaraujo do you think we want to test that as FT test and/or add a check for expires?

I think we don't check that in our code as well.

I believe there's validation done in the model here, which is defined for the endpoint here.

If that is indeed the case we can just add this in the improvements task.

KAUTH · 2023-06-26T19:01:33Z

tests/bdd/tokens/test_generate.py

+    return response
+
+
+@then("the admin should get status code '422'")


Isn't this the same as

repository-service-tuf-api/tests/bdd/tokens/test_generate.py

Line 106 in ddb8f74

@then("the admin should get status code '422'")

?

tests/bdd/features/tokens/generate.feature

kairoaraujo · 2023-07-29T12:01:37Z

About this PR, as I discussed with @KAUTH at EuroPython and also with @MVrachev before

Should we merge it as an "as is" from the FT in the umbrella and work on specific issues for improvement?
Unless there is a must to fix the comments above.

kairoaraujo · 2023-07-29T12:04:47Z

Rebased

KAUTH · 2023-07-30T09:00:42Z

Should we merge it as an "as is" from the FT in the umbrella and work on specific issues for improvement?
Unless there is a must to fix the comments above.

Sounds good to me, we can create an issue to work on the comments left in this PR. Also need the tests to succeed before merging.

kairoaraujo · 2023-07-31T11:22:58Z

Sounds good to me, we can create an issue to work on the comments left in this PR. Also need the tests to succeed before merging.

Fixed the dependencies and tests run.

kairoaraujo · 2023-08-03T13:29:42Z

Rebased

MVrachev

I created an issue where I tried to link to the improvements comments: #398
@KAUTH please have a look if they represent your suggestions accurately or if am I missing something.

I only have two questions.
When they are answered I think we are good to go.

MVrachev · 2023-08-03T16:13:18Z

tests/bdd/features/tokens/login.feature

I think we don't check that in our code as well.
We relly that

repository-service-tuf-api/repository_service_tuf_api/token.py

Line 212 in 3f8409c

expires = datetime.fromtimestamp(token.get("exp"))

will fail.

@kairoaraujo do you think we want to test that as FT test and/or add a check for expires?

MVrachev · 2023-08-03T17:11:43Z

tests/bdd/tokens/test_generate.py

+    target_fixture="response",
+)
+def the_admin_sends_with_invalid_expires_in_payload(
+    test_client, headers, payload


@kairoaraujo am I correct here?

KAUTH · 2023-08-03T20:22:38Z

@KAUTH please have a look if they represent your suggestions accurately or if am I missing something.

@MVrachev I double-checked, you got them all, thanks!

kairoaraujo · 2023-08-17T06:37:43Z

Rebased

The current BDD Tokens tests are part of integration tests in the RSTUF umbrella repository. This BDD feature test doesn't belong to the integration tests once API Token authentication/authorization (auth) feature is a single component. The BDD test is still valid once it describes very well the expected behavior. This commit adds the test to the API and is part of the repository-service-tuf/repository-service-tuf#41 Closes repository-service-tuf#281 Signed-off-by: Kairo de Araujo <[email protected]>

Fix the tests for tests workflow Signed-off-by: Kairo de Araujo <[email protected]>

MVrachev · 2023-08-17T10:45:42Z

@KAUTH I cannot merge until you give approval as well.
It's showing that 1 person requires changes.

kairoaraujo requested review from lukpueh, KAUTH and MVrachev June 20, 2023 08:18

kairoaraujo force-pushed the tests_bdd_tokens branch from 1f5025e to ddb8f74 Compare June 20, 2023 09:22

kairoaraujo mentioned this pull request Jun 20, 2023

Remove username prompt rstuf admin login repository-service-tuf/repository-service-tuf-cli#303

Merged

This was referenced Jun 20, 2023

Remove Auth from FT repository-service-tuf/repository-service-tuf#363

Closed

Remove auth from FT repository-service-tuf/repository-service-tuf#364

Merged

MVrachev reviewed Jun 26, 2023

View reviewed changes

KAUTH requested changes Jun 26, 2023

View reviewed changes

kairoaraujo self-assigned this Jul 11, 2023

kairoaraujo force-pushed the tests_bdd_tokens branch from ddb8f74 to 10074d1 Compare July 29, 2023 12:04

kairoaraujo requested a review from KAUTH July 31, 2023 11:23

MVrachev added this to the v0.6.0b1 milestone Aug 3, 2023

kairoaraujo force-pushed the tests_bdd_tokens branch from a919a90 to 58dd19f Compare August 3, 2023 13:29

kairoaraujo requested a review from MVrachev August 3, 2023 13:29

MVrachev mentioned this pull request Aug 3, 2023

Task: Improve BDD tests #398

Open

6 tasks

MVrachev reviewed Aug 3, 2023

View reviewed changes

kairoaraujo force-pushed the tests_bdd_tokens branch from 58dd19f to d325510 Compare August 17, 2023 06:31

kairoaraujo requested a review from MVrachev August 17, 2023 06:37

Kairo de Araujo added 2 commits August 17, 2023 12:40

Fix the pytest-bdd after rebase

3dc4390

Fix the tests for tests workflow Signed-off-by: Kairo de Araujo <[email protected]>

kairoaraujo force-pushed the tests_bdd_tokens branch from d325510 to 3dc4390 Compare August 17, 2023 10:40

MVrachev approved these changes Aug 17, 2023

View reviewed changes

KAUTH approved these changes Aug 17, 2023

View reviewed changes

kairoaraujo merged commit 7597519 into repository-service-tuf:main Aug 17, 2023
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BDD tests for Token Feature to RSTUF API #369

Add BDD tests for Token Feature to RSTUF API #369

kairoaraujo commented Jun 20, 2023

codecov bot commented Jun 20, 2023 •

edited

Loading

kairoaraujo commented Jun 22, 2023

MVrachev Jun 26, 2023

MVrachev Aug 3, 2023

KAUTH Jun 26, 2023

KAUTH Jun 26, 2023

KAUTH Jun 26, 2023

KAUTH Jun 26, 2023

MVrachev Aug 3, 2023

KAUTH Aug 3, 2023 •

edited

Loading

KAUTH Jun 26, 2023

kairoaraujo commented Jul 29, 2023

kairoaraujo commented Jul 29, 2023

KAUTH commented Jul 30, 2023

kairoaraujo commented Jul 31, 2023

kairoaraujo commented Aug 3, 2023

MVrachev left a comment •

edited

Loading

MVrachev Aug 3, 2023

MVrachev Aug 3, 2023

KAUTH commented Aug 3, 2023

kairoaraujo commented Aug 17, 2023

MVrachev commented Aug 17, 2023

		"""Admin uses HTTP API to generate a token."""


		@given("the admin has the admin password", target_fixture="admin_passwd")

		return response


		@then("the admin should get status code '422'")

Add BDD tests for Token Feature to RSTUF API #369

Add BDD tests for Token Feature to RSTUF API #369

Conversation

kairoaraujo commented Jun 20, 2023

codecov bot commented Jun 20, 2023 • edited Loading

Codecov Report

kairoaraujo commented Jun 22, 2023

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

KAUTH Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kairoaraujo commented Jul 29, 2023

kairoaraujo commented Jul 29, 2023

KAUTH commented Jul 30, 2023

kairoaraujo commented Jul 31, 2023

kairoaraujo commented Aug 3, 2023

MVrachev left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KAUTH commented Aug 3, 2023

kairoaraujo commented Aug 17, 2023

MVrachev commented Aug 17, 2023

codecov bot commented Jun 20, 2023 •

edited

Loading

KAUTH Aug 3, 2023 •

edited

Loading

MVrachev left a comment •

edited

Loading