-
Notifications
You must be signed in to change notification settings - Fork 22
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
test(e2e): add health check and functional tests #110
test(e2e): add health check and functional tests #110
Conversation
I suggest to add a link to the pr of the "assets" repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaschmid @irinamesh Contrary to what I said in todays daily, I didn't create directly license review requests at the EF IP team for the new dependencies in this PR, but I did pre-vet them, and did find some significant issues:
nuget/nuget/-/RestAssured.Net/2.8.1 --> not OK because tagged or release sources are missing in source code repo, can you please find an alternative (also necessary due to next line)?
nuget/nuget/-/Newtonsoft.Json.Schema/3.0.15 --> (dependency of RestAssured.Net) not OK: GNU Affero General Public License or Commercial License. This is incompatible with our project.
nuget/nuget/-/Bitmap.Net/1.0.1 --> not OK because tagged or release sources are missing in source code repo, but 1.0.1 appears at least to be the latest version so it COULD be OK for the team IP to upload the current state from master to the IP issue. But that's just could, is there an easy alternative?
nuget/nuget/-/HtmlAgilityPack/1.11.46 --> OK
nuget/nuget/-/SixLabors.ImageSharp/3.0.1 --> OK
nuget/nuget/-/Stubble.Core/1.10.8 (dependency of RestAssured.Net) --> OK
@evegufy: Thanks for the hints. For For |
Is that already sufficient for that topic? Intermediate result for the license of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@irinamesh I just ran dotnet format and there are a couple of issues:
https://github.com/eclipse-tractusx/portal-backend/actions/runs/5461453783/jobs/9945408008
Could you please solve those issues?
You could try solving those issues by running:
dotnet format .\src --no-restore
If that doesn't work, you could investigate with:
dotnet format .\src --verify-no-changes --no-restore
Sonarcloud also revealed 2 Code Smells, could you please solve them as well?:
https://sonarcloud.io/project/issues?id=eclipse-tractusx_portal-backend&pullRequest=110&resolved=false&types=CODE_SMELL
@irinamesh good! there are now much less findings from dotnet format. Could you please take care of the remaining ones?: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move the entire EndToEnd Testing Project out of the src and into the tests directory, ideally into a 'endtoend' directory.
Please also comply with our namespace naming convention, which in this case would be something like 'Org.Eclipse.TractusX.Portal.Backend.EndToEnd.Tests'
@@ -49,4 +49,4 @@ jobs: | |||
- name: Check Format | |||
run: dotnet format src --verify-no-changes --no-restore | |||
- name: Test | |||
run: dotnet test src --no-restore --verbosity normal | |||
run: dotnet test src --filter FullyQualifiedName!~EndToEnd --no-restore --verbosity normal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this locally (as the workflow doesn't current get to this step due to the dotnet format issue) and I'm generally a bit skeptical if this filter will work, but I think it won't work for sure without escaping of '!' https://learn.microsoft.com/en-us/dotnet/core/testing/selective-unit-tests?pivots=mstest#character-escaping and without defining the proper namespace.
I'd suggest to change to this:
dotnet test src --filter FullyQualifiedName\!~EndToEnd.Tests --no-restore --verbosity normal
Or respectively to dotnet test src --filter FullyQualifiedName\!~Org.Eclipse.TractusX.Portal.Backend.EndToEnd.Tests --no-restore --verbosity normal
once the proper namespace naming convention is used.
I created the following IP issues: The IP Issue for nuget/nuget/-/Namotion.Reflection/2.1.2 wasn't created yet due to open issue for the source: RicoSuter/Namotion.Reflection#131 |
@irinamesh Here is the first workflow run from the fork: https://github.com/catenax-ng/tx-portal-backend/actions/runs/5486098873 cc: @jjeroch |
I adjusted the ReportPortal config stuff, but cannot re-trigger the test case. Even though, most tests run successfully, I think 2-4 should not fail normally. Irina and I will have a look at them tomorrow. |
@aaschmid just fyi, I created the IP Issue for nuget/nuget/-/Namotion.Reflection/2.1.2 and uploaded to current state of the release branch as tag wasn't pushed up to now. |
@aaschmid @irinamesh I did a test run with the current state https://github.com/catenax-ng/tx-portal-backend/actions/runs/5517320395 |
Thanks for the run and the good news, we will have a look at the details tomorrow. |
Just an FYI for @aaschmid and @evegufy: RestAssured .Net now has an official 4.0.0 release, so you won't need to use a beta anymore: https://www.nuget.org/packages/RestAssured.Net/4.0.0 |
@irinamesh I was able to check regarding assertions, could you please change to fluent assertion? as this appears to be our mainstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@irinamesh as just discussed, we do have license issue
Regarding connection to the ReportPortal and test results in it it works as expected, but i need to check why registration scenarios fail. I'll be back with an update on this issue till Friday. |
@irinamesh is there a reason you extracted the deserialization of response payloads in the E2E tests into a separate class in f1a1b7f and 84ff27e ? Not because I want to question your decision, but because RestAssured does support (de-)serialization using Just curious to see if there's anything that you think should be improved in RestAssured .Net, that's all! Thanks :) |
Tag for Namotion.Reflection/2.1.2 has been pushed and IP issue has been updated https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/9320#note_1181119 |
@basdijkstra hi and first thank you for a great framework! I really enjoy working with it as it covers a lot of functionality and really straight forward for understanding!
|
As discussed yesterday I implement the following changes:
|
Hey @irinamesh, the challenge here is that you're using I think But I can't guarantee that you'll be able to do exactly what you want with I've raised basdijkstra/rest-assured-net#99 to remind myself to dive deeper into this when I return from my holidays. Feel free to add a comment there if you've got specific ideas or suggestions around this. Maybe at some point I'll switch from Thank you so much for your explanation, gives me some food for thought. And thank you, too, for the kind words. It means a lot to me seeing people use this library in their projects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BUG
Please check the report portal - https://portal-reportportal.dev.demo.catena-x.net/ui/#superadmin_personal/launches/all/12/330/332/log
@jjeroch due to test names, I can create something like I also might have found a solution for not prefixing by namespace and test class names (here Would this satisfy your requirements? And if yes, do you have any preference of the names? |
$"Bearer {ClearingHouseUserToken}") | ||
.When() | ||
.Body( | ||
"{\"callbackUrl\":\"https://portal-backend.dev.demo.catena-x.net/api/administration/registration/clearinghouse\",\"participantDetails\":{\"name\":\"SmokeTest CH\",\"city\":\"Stuttgart\",\"street\":\"Test Street\",\"bpn\":\"BPNL000SMOKE0011\",\"region\":\"Bavaria\",\"zipCode\":\"01108\",\"country\":\"Germany\",\"countryAlpha2Code\":\"DE\"},\"identityDetails\":{\"did\":\"did:sov:RPgthNMDkVdzYQhXzahh3P\",\"uniqueIds\":[{\"type\":\"local\",\"value\":\"HB8272819\",}]}}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use ClearinghouseTransferData
and deserialize it to make sure we always send correct data and see it as soon as the implementation changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the url should also not be part of the code, such that I will move it to the environment variables.
Also, this contains some IdentityDetails
for which I want to ask, if that is ok to keep them within the code directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jjeroch could you please take a look at this? As per initial requirements body could be hardcoded in this case. As we don't use hardcoded URLs anymore it makes sense to define it with an environment variable. Can IdentityDetails be defined directly or should it also be moved from the code?
@irinamesh as discussed this morning here a new workflow run: https://github.com/catenax-ng/tx-portal-backend/actions/runs/5587068138 All dependencies are approved now :) could you please download the updated DEPENDENCIES file from the workflow artifact here and commit it (replace of existing DEPENDENCIES file on root level)? |
@aaschmid thank you @jjeroch here are screenshots that show how can it look in ReportPortal: Test scenarios without test data: Do you have any suggestions for improvement? |
Yes, looks broken also by me. As I see it affects only this page and reload does not help. If it is needed, we can create an issue here: https://github.com/reportportal/reportportal/issues, but I'm not sure if they can reproduce this. |
@basdijkstra thank you too! yes, we want to be on the safe side, that's why I've decided on this implementation. We'd love to hear about updates and new releases. Have a good holiday! You're doing a great job! |
@irinamesh thanks. Lets take "Test scenarios without test data" the test data itself should be stored in the logs (means when clicking on the Test Scenario and looking up the details. |
@irinamesh @jjeroch here the 1rst run with int env https://github.com/catenax-ng/tx-portal-backend/actions/runs/5630519459 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closed the fixed issues, thanks for that @aaschmid
found one minor additional point. there are two open comments left, besides of that everything looks fine.
Is it planned to merge the fluentAssertion branch into this one?
@Phil91 thanks for your feedback. I now also cherry-picked my changes due to fluent assertion and pushed it here - just didn't want to interrupt the review process with a lot of new changes -, see commit hash d8d9f2b. The remaining cherry-pick is about test names which I also prepared as separate branch to cherry-pick from. My first version was already reviewed, and I incorporated the changes in here. Remaining question is about the displaying of test data, see first commit and comments there. |
tests/endtoend/NotificationInitScenario/ModifyCoreUserRoleScenario.cs
Outdated
Show resolved
Hide resolved
…itHub actions Update dependencies file with source: Workflow https://github.com/catenax-ng/tx-portal-backend/actions/runs/5587068138 Contained commits: CPLP-2513: add RestAssured sample test CPLP-2513: test RestAssured for notification controller CPLP-2513, CPLP-2591: execute RestAssured tests for Notification endpoint in order CPLP-2513, CPLP-2591: make test independent of number of notifications CPLP-2513, CPLP-2591: add secrets to test github action CPLP-2513, CPLP-2591: put secrets in shared project CPLP-2513, CPLP-2592: add tests for get requests with applicationId CPLP-2596: add tests for get requests in Administration.Service CPLP-2596: add happy path descriptions CPLP-2596: test for upload doesn't work properly CPLP-2596: add tests for get and post requests of administration.service for happy path CPLP-2513, CPLP-2592: structure e2e test scenarios CPLP-2596: add functions to get applicationId and companyId, delete ApplicationId in Secrets CPLP-2513, CPLP-2592: send invitation email to temp service and fetch password message CPLP-2513, CPLP-2592: upload document CPLP-2513, CPLP-2592: fine-tuning of get requests CPLP-2513, CPLP-2592: delete unused libraries CPLP-2513: test TempMailApi, first get request CPLP-2513: add check of mailbox and create of hashed email-address CPLP-2513: add fetch password CPLP-2596: add test for submit registration, adjust get requests CPLP-2513: add fetch password with developermail CPLP-2513, CPLP-2592: add check of status for get application details CPLP-2513, CPLP-2593: add submit registration test with 403 status code CPLP-2596: add fetch password from devMail, set application status, set order of tests CPLP-2596: add submit registration test with expectation of 403 status code and necessary get requests CPLP-2513, CPLP-2596: authorization flow without password update CPLP-2596: add validation of the application status for post requests CPLP-2513, CPLP-2596: add html decode of password CPLP-2513, CPLP-2596: authorization flow with password update for new invited user CPLP-2727: validate in-registration-user-invite CPLP-2724: add GetCompanyDetailData returns valid companyDetailData CPLP-2592: deserialize json bodies CPLP-2513, CPLP-2596: add html decode of password for temp-mail CPLP-2725: add get available roles and consents for SubmitCompanyRoleConsentToAgreements CPLP-2724: add verifikation of correct storage of data company detail CPLP-2728: add test for update company detail data CPLP-2725: add get company roles and consents for selected roles CPLP-2723: add test data with company details and company roles CPLP-2723: add test data helper to get data from test data files CPLP-2596: change regEndpointHelper and operatorToken to static CPLP-2714: add modify core user roles CPLP-2723: add fetch test data file for registration without bpn CPLP-2596: add secrets for tech users CPLP-2596: add parameters for GetAccessToken function to get token for selected user CPLP-2714: add test data file for modify core user roles CPLP-2596: add check of applicationID by getting application details CPLP-2714: change variables name CPLP-2715, CPLP-2716: add sdfactory and wallet health check (wip) CPLP-2802: add bpdm interface health check CPLP-2803: add clearinghouse interface health check (wip) CPLP-2723: add scenario test with test data sets from file CPLP-2596, CPLP-2723: add use of test data for happy paths CPLP-2832 CPLP-2833 CPLP-2834 CPLP-2835: add helper functions, add update and delete scenarios CPLP-2723: add test data sets (wip) CPLP-2714: handle multiple role (un)assignments CPLP-2832 CPLP-2833 CPLP-2834 CPLP-2835: add get operator token CPLP-2832: add get token for technical user CPLP-2832: add check if the new service account is added correctly CPLP-2834: add get token with new credentials CPLP-2866: add test data file for multiple run of service account scenarios CPLP-2864: move baseurls and environment to separate class CPLP-2864: add appsettings.json as env CPLP-2864: move operator and techUser company names to appsettings CPLP-2867: add delete created service accounts after test run CPLP-2835: add check availability of service account CPLP-2871: move assertions to scenario test function CPLP-2596: move separate scenarios without bpn to one class CPLP-2582: add ReportPortal packages CPLP-2715: move wallet tests to separate class CPLP-2716: add secrets for int and change urls CPLP-2803: change urls for ClearingHouse CPLP-2883: add test portal user to secrets CPLP-2883: switch to test portal user, add user to db api tests CPLP-2714: remove test data file, add use of random core role CPLP-2879: create e2e test project CPLP-2836: add EndToEndTests CPLP-2879: move system health check tests to e2e test project CPLP-2877: add yml-file for e2e tests CPLP-2877: add missing file CPLP-2877: add schedule CPLP-2870: add create new app scenario (wip) CPLP-2870: add test for upload of app image and test documents CPLP-2596: change logic of fetching password to get password in first p after "below you can find" CPLP-2596: change logic of fetching password for TempMailApi, add comments CPLP-2832 CPLP-2833 CPLP-2834 CPLP-2835: update service account creation description CPLP-2716, CPLP-2834: move hardcoded variables to createNewServiceAccount function, add additional checks CPLP-2879: run tests serially CPLP-2879: remove operator user CPLP-2879: move resources to E2E test project CPLP-2879: remove Reportportal config CPLP-2879: test workflow CPLP-2877: test github workflow CPLP-2879: restructure test data files CPLP-2877: remove authFlow section CPLP-2877: test if secrets are readable CPLP-2877: remove start on push, add env variables CPLP-2935: add portal user token CPLP-2877: add secrets and variables on env level CPLP-2877: move environment from steps to jobs CPLP-2877: remove appsettings values CPLP-2877: move urls to appsettings CPLP-2877: remove URLs from code CPLP-2877: divide tests in health check and functional tests CPLP-2877: run all tests in one job CPLP-2877: switch to FullyQualifiedName CPLP-2877: add directory CPLP-2877: update directory CPLP-2877: continue on error after system health check tests CPLP-2877: move continue on error to health check tests CPLP-2877: remove URL from formContent CPLP-2877: test edit of json config file CPLP-2877: change file name CPLP-2877: exclude end2end tests in unit-tests CPLP-2802: add bpdm url to ressources and appsettings CPLP-2836: rename class, add additional check for token CPLP-2680: add variables for clearing house CPLP-2877: add health check trait CPLP-2877: add urls to health check run CPLP-2877: add bpdm url CPLP-2714: switch to portal user CPLP-2596: rename DbApiTest in RegistrationHC CPLP-2879: group tests in traits CPLP-2879, CPLP-2877: add dropdown for test category CPLP-2877: rename file and workflow CPLP-2596: delete countryDe variable CPLP-2877: remove bitmap CPLP-2877: resolve merge conflicts CPLP-2877: install reportportal.xunit CPLP-2870: switch to deserialize data by language tags CPLP-2714: add assert messages remove Newtonsoft.Json from E2E tests CPLP-2832: add assert message CPLP-2877: move reportportal url to secrets, add continue on error for registration scenario CPLP-2877: correct typo CPLP-2877: change comment CPLP-2877: correct typo CPLP-2877: add url for report portal CPLP-2596: call companyRoleAgreementData endpoint to get agreement ids CPLP-2596: change document path to document name CPLP-2879: add collections, disable test parallelization CPLP-2596: correct type of document CPLP-2877: apply dotnet format CPLP-2877: apply dotnet format CPLP-2877: add console writer and response log level on error CPLP-2877: move project to test directory, rename namespace CPLP-2877: apply dotnet format CPLP-2832: move delete of accounts into scenarios CPLP-2877: add response log level on error CPLP-2596: add first draft of registration scenario with bpn (hardcoded roles) CPLP-2877: rename secrets for interface partner health check, add clearing house token url CPLP-2877: refactoring - move retrieve get token to a separate class CPLP-2877: update RestAssured version to 4.0.0-beta.1 CPLP-2877: resolve migration to new rest assured version conflict, use fluent assertions CPLP-2877: upgrade HtmlAgilityPack CPLP-2803: add assertion CPLP-2877: remove tech company name from appsettings CPLP-2832: remove not necessary null checks CPLP-2596: adjust exception messages, remove hardcoded url CPLP-2877: rename system to portal health check CPLP-2877: add abstract class to overwrite console output CPLP-2877: update RestAssured version CPLP-2877: set console output to all scenarios CPLP-2877: refactoring - move data deserialization to separate class CPLP-2596: fix deserialization issue after migration to RestAssured 4.0.0 CPLP-2596: adjust password search CPLP-2870: remove image library, adjust flow to use only test data CPLP-2877: add execution of all tests as separate step CPLP-2596: add test data file for registration with bpn Enhance create app test scenario with additonal lead image type - also refactored to a common method for all document types Signed-off-by: Andreas Schmid <andreas.schmid@tngtech.com>
@Phil91 as discussed today, I rebased and squashed everything onto current The squash also contains the small changes requested by Julia. Unfortunately, I squashed them as well such that I will describe them shortly here. If that is not sufficient please let me now:
|
Signed-off-by: Andreas Schmid <andreas.schmid@tngtech.com>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
add following e2e health check and functional tests: Interface Partner Health Check (BPDM, ClearingHouse, SdFactory, Wallet), System Health Check (DB API tests and BaseDataLoadCheck), Notification init scenario, Registration scenarios, Service account CUD scenarios, Create app scenario. as well as a GH workflow for manual trigger.
add following e2e health check and functional tests: Interface Partner Health Check (BPDM, ClearingHouse, SdFactory, Wallet), System Health Check (DB API tests and BaseDataLoadCheck), Notification init scenario, Registration scenarios, Service account CUD scenarios, Create app scenario. as well as a GH workflow for manual trigger.
add following e2e health check and functional tests: Interface Partner Health Check (BPDM, ClearingHouse, SdFactory, Wallet), System Health Check (DB API tests and BaseDataLoadCheck), Notification init scenario, Registration scenarios, Service account CUD scenarios, Create app scenario. as well as a GH workflow for manual trigger.
add following e2e health check and functional tests: Interface Partner Health Check (BPDM, ClearingHouse, SdFactory, Wallet), System Health Check (DB API tests and BaseDataLoadCheck), Notification init scenario, Registration scenarios, Service account CUD scenarios, Create app scenario. as well as a GH workflow for manual trigger.
add following e2e health check and functional tests: Interface Partner Health Check (BPDM, ClearingHouse, SdFactory, Wallet), System Health Check (DB API tests and BaseDataLoadCheck), Notification init scenario, Registration scenarios, Service account CUD scenarios, Create app scenario. as well as a GH workflow for manual trigger.
Description
Add following e2e health check and functional tests:
As well as a GH workflow for manual trigger.
Documentation
PR for documentation: eclipse-tractusx/portal-assets#55
Why
We were asked to create first e2e health check and functional tests with RestAssured in order to have a better safety net and to avoid regressions.
Issue
Link to Github issue: not available.
Checklist