search

apinf / platform

Apinf - Open source API management platform with multi proxy and protocol support
https://apinf.com/
European Union Public License 1.1
74 stars 33 forks source link

Proposal - .test folder in apinf/platform should be deprecated and content moved to apinf-testing repo #2622

Closed as33ms closed 7 years ago

as33ms commented 7 years ago

Hello all, @apinf/developers @apinf/architects @apinf/core @apinf/testing

In order to speed up the process of adding test cases for the product, I would like to propose that all the "Test" development is done in a separate repository (which already exists btw at apinf-testing). This will also be inline to the convention that we have a separate "docs" repository. The idea is that the tests that are being written should land up in the repos and run in CI in rather quick feedback cycles than the actual process of development where everything in the platform repo goes into develop only through pull requests.

As for honoring the "tests" folder in apinf/platform is concerned, we can have that as a git submodule to improve maintainability.

Your thoughts?

mauriciovieira commented 7 years ago

@ashakunt I removed .tests in https://github.com/apinf/platform/pull/2522 ( commit https://github.com/apinf/platform/pull/2522/commits/4da4017e5ea217595a13e340dde0cf996b311b78) Please read that pull request. It is ready for review.

mauriciovieira commented 7 years ago

In my opinion,

  1. we should have tests/ directory (as in #2522, I hope it get merged) for end-to-end and (future) server-side unit tests.
  2. we could use https://github.com/apinf/apinf-testing (maybe renamed as platform-testing) for stress, and security, and any other non-functional test suites.

I disagree with having functional tests (either unit, integration, end-to-end, acceptance, anything that tests function) far from the business logic (the actual repository, platform). I don't see it as a good practice. This project tried before, too, and it did not work.

What we need after #2522 is to make sure the test suit is run after each commit, and I know it's possible since Rocket.Chat is using a similar architecture and is using travis-ci.

mauriciovieira commented 7 years ago

And finally, I don't see any advantage on having tests/ directory as a submodule. This makes the code review process more difficult, since the reviewer would not see immediately that new tests where added for new features, for instance (they would have to make sure equivalent commits where done on the separate repository).

philippeluickx commented 7 years ago

Personally agree with @mauriciovieira . Tests should be part of the developers deliverables and thus remain close to the code they write. Unit tests should stay in apinf/platform. Performance and security testing could go to a seperate repo. E2E testing is debatable for me.

@ashakunt if I understand correctly your proposal is because it takes to long for tests to arrive in the develop or master branch, because they need to go through PRs? I would argue that tests should also go through a code-quality review and thus follow a similar process. However, if anyone has an idea to speed it up that is agreeable for everyone, that'd be awesome.

as33ms commented 7 years ago

Thanks, @philippeluickx and @mauriciovieira for your comments. I will check #2522 in a while. Any piece of code that will run in production will pass through standard review processes. Any piece of code that tests the production code will have to pass through review process as well.

So, both, development and testing will be peer reviewed. Having said that, rapid development is taking place for the product and many features are already there, but there are no tests for them.

For the coming few sprints, there will be lot of things going on for both, "tests development" and "feature development". I would personally like the tests for existing features to start running in CI as soon as possible and all new features to accompany the test cases.

Keeping all this in mind, @philippeluickx understood my emotions that I want the tests to land at their_respective_places as soon as possible. I am not sure if that implies that they should not go through PRs?

... more to come...

as33ms commented 7 years ago

I agree that as a developer, it makes sense to have tests as a part of the deliverable and the PR. So, I would vote against my own proposal.

as33ms commented 7 years ago

Hence, the consensus so far, we have had is that we will have all the tests under the tests directory. Further, some of my comments w.r.t. other comments:

I disagree with having functional tests (either unit, integration, end-to-end, acceptance, anything that tests function) far from the business logic (the actual repository, platform). I don't see it as a good practice. This project tried before, too, and it did not work.

IMHO, Unit testing and Functional testing allow the development teams to act on potential regressions and defects quite early on. The situation might change based on the project but otherwise, they are of great importance and are a very good practice.

What we need after #2522 is to make sure the test suit is run after each commit, and I know it's possible since Rocket.Chat is using a similar architecture and is using travis-ci.

An example of Rocket.Chat might be good, but we also have to understand that for tests to run on every commit and provide a fast feedback, the test have to run not more than 2-3 minutes. Here again, unit tests and some times, functional tests start to show their importance.

The tests that we are going to run are "End to end" tests or "Integration tests". Eventually, this test set will grow and they will take sometimes 15-20 minutes or more to complete. As a developer, tell me if you really want to wait that long for every commit?

Unfortunately, with the platform we have chosen for development, unit tests or functional tests are not so easy to run. So, we have to carefully choose the set of test cases that are run at what time. I already have a 75% proposal ready for that and will present to the team as soon as I can.

philippeluickx commented 7 years ago

Good points there. There is a few seperate concerns:

  1. Catching up on tests: I am OK with taking a few shortcuts in order to get tests (that should have been there already) faster in develop/master branches. Feel free to provide a proposal. This needs an OK from @bajiat .
  2. Efficient workflow: I feel that there is a lot to testing and at least I feel very unstructured in my mind about this. Do we have unit tests? Functional? E2E? Integration? Performance? Security? If someone with the knowledge could draft up a document to get us all on the same page, taking the current context (meteor) into account - that would be great. We also need to define responsabilities (e.g. a feature needs to be delivered with unit tests, but not E2E? Who does then the E2E?). Totally agree with you that a dev can not wait 20 minutes for test results.
  3. Goals: do we aim for 100% test coverage? How do we get there? Proposal I have is that
    • Any bug gets fixed at least in the next sprint (yes, even silly ones). But they need a failing test case first.
    • Every sprint there is 1 dev responsible for closing bugs in the backlog (I think this was already discussed, not sure if it happens though)
as33ms commented 7 years ago

@philippeluickx : I will share a document during this week about what kind of coverage and what kind of tests.

mauriciovieira commented 7 years ago

Every sprint there is 1 dev responsible for closing bugs in the backlog (I think this was already discussed, not sure if it happens though)

@philippeluickx I disagree with this '1 dev' rule. Has this rule ever happened ?

philippeluickx commented 7 years ago

@mauriciovieira There's been talks about it, no execution AFAIK. Any proposals on getting bugs closed faster?

brylie commented 7 years ago

Any proposals on getting bugs closed faster?

A basic bug triage process might improve our bug response/closure time.

Since this is somewhat off-topic for the current issue, I will open a new issue where we can discuss/design a bug triaging process for our team.

mauriciovieira commented 7 years ago

.test directory is no more, and https://github.com/apinf/apinf-testing has the contents of the old .test directory, so this issue is closed