Include unit tests for micro services

[suryajak]

Description of the Change

Include unit tests for micro services so that they run as part of the PR process. Also fixed broken unit tests.

Benefits

Catch issues early during the review process.

Possible Drawbacks

Time for automated PR checks will increase

Applicable Issues

[devsatishm]

Let's merge them to events before merging them to master.

[suryajak]

I am not sure I follow why this would go to events branch? This is an independent feature that has been validated as a stand alone and it has no relation to events.

[bleggett]

For failing tests, my preference would be to either fix them or delete them entirely.

Disabling is usually the worst option.

[suryajak]

We have an issue added here to address the broken unit test. If the question is about dead code (commented out unit test), I can swing either way.

[bleggett]

If the logged issue will be picked up in the current or next sprint, then we can leave as is, otherwise would prefer they be deleted.

[devsatishm]

I am not sure I follow why this would go to events branch? This is an independent feature that has been validated as a stand alone and it has no relation to events.

How did we test these changes? Did they go into any of the feature branch (new is fine too) and got validated? We'll need to avoid merging changes to master directly. Intention behind asking them to be merged to events branch is to have these changes validated end to end as part of daily stack validation.

[suryajak]

This PR is only for integrating and fixing unit tests. By design unit tests don't need the full stack to come up (except for when you are refactoring the code against which the unit tests are running, which isn't the case here). The validation was done locally as well as by travis as part of the PR.

[devsatishm]

This PR is only for integrating and fixing unit tests. By design unit tests don't need the full stack to come up (except for when you are refactoring the code against which the unit tests are running, which isn't the case here). The validation was done locally as well as by travis as part of the PR.

Sure, agreed. But that doesn't qualify these changes to go to master branch directly. Irrespective of the changes, it is required for all them to go into a non-master branch (hotfix-*, feature-*, patch-*, develop) first and then get merged into master. Changes cannot go directly from a developer's fork to the master branch.

[suryajak]

Is this a new policy change that all changes have to go through a non-master branch? #582

[devsatishm]

Is this a new policy change that all changes have to go through a non-master branch? #582

There is no policy change. This is the development workflow that this project follows. And, like any others, there can be exceptions like fixing high priority security vulnerabilities in code flagged by Github or hot-fixes that can't wait for the entire development lifecycle to complete. Fixing unit tests doesn't belong to this category. Changes in #582 are tested in the daily stack.