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

feat(schematic): integration testing #2400

Merged
merged 11 commits into from
Dec 15, 2023
Merged

feat(schematic): integration testing #2400

merged 11 commits into from
Dec 15, 2023

Conversation

andrewelamb
Copy link
Contributor

@andrewelamb andrewelamb commented Dec 8, 2023

This is identical to #2398 plus some fixes to the testing workflow.

This PR chas a new wrokflow that is trggred when PRs are made from the new Schematic-API-Staging branch into main.

This PR:

  • fixes fds-1288
  • fixes fds-1288
  • PRs are now done from forks to Schematic-API-Staging instead of main
  • PRs to Schematic-API-Staging only trigger tests that don't require secrets
  • PR's are then don form Schematic-API-Staging to Main
  • A new workflow that triggers when PRs are done from Schematic-API-Staging to Main that runs all tests has been added
  • All non synapse tests are not mocked at all
  • A new set of tests test_synapse_endpoints.py has tests for all synapse endpoints where nothign is mocked
  • handle_exceptions is now tested directly
  • When the schema url isn't correct an InavlidSchemaURL exception is now raised
  • handle exceptions returns a 404 status when InavlidSchemaURL is caught

* changed authenticication so that only endpoints that need it have it

* updated schematic

* add patch for access token

* schema endpoints no longer mockeed

* added tests for handle exceptions

* added integration tests

* marked synapse tests

* added error handling for bad schema urls

* fix error message

* add workflow for end to end testing

* fix some test results

* add unit mark

* add unit mark

* add workflow for testing with secrets

* rename file

* fix synapse test file when secrets file doesnt exists

* fix test workflows

* turned synapse ids into secrets in workflow

* turned synapse ids into secrets in workflow
@andrewelamb
Copy link
Contributor Author

@tschaffter @GiaJordan Can I get your approval on this?

Copy link

@GiaJordan GiaJordan left a comment

Choose a reason for hiding this comment

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

approving per discussion on slack with @andrewelamb

Copy link
Member

@tschaffter tschaffter left a comment

Choose a reason for hiding this comment

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

  • Can you please add a high-level description of what this PR is doing? When is this workflow being triggered?
  • Why does the title of the PR includes "(feat(schematic): integration testing #2398)"?
  • The new GH workflow duplicates parts of the CI workflow common to all projects. This would lead to a waste of resources. Based on previous discussions, you may want this PR to focus on integration testing for Schematic. Also, now or in another PR, you could consider running it AFTER the common CI workflow successfully passes, so that e2e testing is not run if the linting check fails, for instance.

@andrewelamb andrewelamb changed the title feat(schematic): integration testing (#2398) feat(schematic): integration testing Dec 13, 2023
@andrewelamb
Copy link
Contributor Author

andrewelamb commented Dec 13, 2023

@tschaffter

Can I get rid of the push job in the new workflow? That seems redudant.

"The new GH workflow duplicates parts of the CI workflow common to all projects. This would lead to a waste of resources."

What parts cna I get rid of in the pr job in the new workflow? My understanding is almost all of it is needed to for the e2e testing to work. I could get rid of the Lint the affected projects step since the main CI workflow will check that, and the Set up Renv cache since the API won't use that. What other steps can go?

@andrewelamb andrewelamb merged commit dad6440 into main Dec 15, 2023
7 checks passed
@andrewelamb andrewelamb deleted the Schematic-API-Staging branch December 15, 2023 18:57
@andrewelamb andrewelamb restored the Schematic-API-Staging branch December 15, 2023 19:00
@andrewelamb andrewelamb deleted the Schematic-API-Staging branch January 3, 2024 20:58
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