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

Add testing suite with coverage of the REST API #262

Open
dymurray opened this issue Mar 9, 2023 · 3 comments
Open

Add testing suite with coverage of the REST API #262

dymurray opened this issue Mar 9, 2023 · 3 comments
Assignees
Labels
sprint 1 testing triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@dymurray
Copy link
Contributor

dymurray commented Mar 9, 2023

Summary

Borrowing the goals from #241, we want to implement regression testing for the hub API by providing a suite of tests that will guarantee the hub conforms to the published API. This suite should be extensible so that new features can include test coverage for any API changes.

The details of this suite should be published in enhancements as follow-ups to konveyor/enhancements#98.

Proposed Design

The API tests should be configurable to run disconnected (outside of a cluster) or connected. When running disconnected, tests requiring a cluster will be omitted:

  • Run tasks.
  • List/Get addon

Packages

  • test/ - Provides tests.
  • test/api/ - Provides API tests.
  • /test/api/resource/ - Each resource (package) is a top level resource that roughly equals an API endpiont.
  • /test/api/fixtures/ - Fixtures. Mainly imported from hub/api and hub/addon to begin with.
  • test/binding/ - Provides addon binding tests.
  • test/TBD - Future.

Example packages:
/test/api/application - contains one (or more .go files) Ex: api_test.go

Fixtures

The test/api package will provide fixtures (mainly imported from other packages in the hub).

  • Resources (objects) from the api package.
    • api.Application
    • api.Tag
    • api.Task
    • api.JobFunction
    • ...
  • Client - REST client (imported from addon binding).
  • Param - Support query parameters (imported from addon binding).
  • Params - Support URL parameters (imported from addon binding).

Example test/api/fixtures/client.go


var (
    // Client is the global configured client.
    Client *addon.Client
)

func init() {
    Client = addon.NewClient(settings.Addon.URL, settings.Addon.Token)
}

Example usage: test/api/application/api_test.go:

import (
    api github/konveyor/tackle2-hub/test/api/fixtures
) 

var (
    client = api.Client
}

func TestApplication() {
    //
    // Create
    created := &api.Application{
        Name: "Test",
        Description: "My test."
        ....
    }
    err := client.Post(api.ApplicationsRoot, created)
    //
    // Get
	found := &api.Application{}
	path := api.Params{api.ID: created.ID}.inject(api.ApplicationRoot)
	err = client.Get(path, found)
    //
    // Update
    created.Description = "Updated"
    err = client.Put(path, created)
    //
    // Get
	found := &api.Application{}
	path := api.Params{api.ID: created.ID}.inject(api.ApplicationRoot)
	err = client.Get(path, found)
    //
    // Delete
    err =client.Delete(path)
}
@aufi aufi self-assigned this Mar 9, 2023
@jortel jortel added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Mar 9, 2023
@jortel jortel moved this to In Progress in Core Mar 13, 2023
@jortel jortel added this to Core Mar 13, 2023
@aufi
Copy link
Member

aufi commented Mar 14, 2023

👍 on the test directory/package structure, other notes below.

Disconnected/in-cluster tests

I'd consider keep connected (cluster-only) tests separated from the disconnected-only REST API tests at least in diferent files since I believe it would test slightly different thing (=>more than the REST API).

Example package structure

Note on api_test.go example structure. TestCreate() vs. TestList() TestGet(), no doubts the test suite needs to cover all methods, but I'm not sure if the proposed structure is beneficial. Would it make more sense to have actions connected together to cover Application lifecycle? Create needs Get to assert returned application data, Get&List need have Application created before, and Delete cleans everything up using again Get to check if it was really deleted..

Rich client

Using the Rich Client with binding around each object's actions seems to me to bring more costs than benefits, details bellow.

  • Creating and maintaining an abstraction on top of the Hub API that would look similar to using Hub models types within the Hub is probably little bit more convenient for Hub developers to read, but is there other advantage doing that?
  • Since the package focuses on API test I don't see as a bad thing to call API methods like err = hub.Post(api.ApplicationsRoot, &r_the_application) (feel free challenge this, it's up for discussion)
  • The test building blocks which I'd see more practical might cover one or more API calls with raising (custom) errors as an input for assertions.
  • Points above are partially caused by a step away from Hub core development I tried to do when started focusing on "Upstream CI" effort to get better overview on Konveyor development/QA/release process, so I'm little afraid if the Hub test suite we build could be re-used by others like QE (not meaning to be only technically possible to be re-used, but to be practical and beneficial for others to re-use it).

Note, I'm not saying I don't want write the code as described above, I can and I'm ready to do that, I just want discuss it/understand thoughts behind it better first.

@jortel
Copy link
Contributor

jortel commented Mar 14, 2023

+1 on the test directory/package structure, other notes below.

Disconnected/in-cluster tests

I'd consider keep connected (cluster-only) tests separated from the disconnected-only REST API tests at least in diferent files
since I believe it would test slightly different thing (=>more than the REST API).

Perhaps. I'm open to doing the most practical thing. I don't anticipate many tests that can only run when connected. So whatever makes sense is fine with me. I had anticipate the Tests that must be connected, would just short-circuit like:

if Disconnected {
    return
}

Example package structure

Note on api_test.go example structure. TestCreate() vs. TestList() TestGet(), no doubts the test suite needs to cover all methods, but I'm not sure if the proposed structure is beneficial. Would it make more sense to have actions connected together to cover Application lifecycle? Create needs Get to assert returned application data, Get&List need have Application created before, and Delete cleans everything up using again Get to check if it was really deleted..

So long as each function tests one thing, I'm fine with the (exported) entry point for the test(s) to be lifecycle oriented. I mainly want to avoid 100+ line functions or functions that are difficult to follow or troubleshoot. Mainly just want good functional decomposition.

Rich client

Using the Rich Client with binding around each object's actions seems to me to bring more costs than benefits, details bellow.

  • Creating and maintaining an abstraction on top of the Hub API that would look similar to using Hub models types within the Hub is probably little bit more convenient for Hub developers to read, but is there other advantage doing that?

The rich client is mainly to provide a simple way to CRUD resources outside of the API test that is testing the endpoint. Outside can mean another test, or outside the Hub repository. For example: The application test(s) may want to create Tags or Business Service. The thinking is that using the rich client to create the Tag or business service would be simple and proven. However the application test itself may not use the rich client to CRUD the application (endpoint being tested).

  • Since the package focuses on API test I don't see as a bad thing to call API methods like err = hub.Post(api.ApplicationsRoot, &r_the_application) (feel free challenge this, it's up for discussion)

That is fine for the API test (testing the application endpoint) but would rather not have that everywhere else that needs to do to create an application for setup/tear-down. That said, I agree the rich client only insulates building the URL and can easily be convinced it's not worth it.
This is mainly a response to Dylan and David wanting re-usable Fixtures.

  • The test building blocks which I'd see more practical might cover one or more API calls with raising (custom) errors as an input for assertions.

Can you elaborate?

  • Points above are partially caused by a step away from Hub core development I tried to do when started focusing on "Upstream CI" effort to get better overview on Konveyor development/QA/release process, so I'm little afraid if the Hub test suite we build could be re-used by others like QE (not meaning to be only technically possible to be re-used, but to be practical and beneficial for others to re-use it).

This seems to be an argument for the Rich Client but suspect I misunderstand.

Note, I'm not saying I don't want write the code as described above, I can and I'm ready to do that, I just want discuss it/understand thoughts behind it better first.

You are making good points and DEFINITELY want these comments. This was meant as a discussion springboard.

@aufi
Copy link
Member

aufi commented Mar 14, 2023

Thanks for your responses, sounds good to me!
👍

(deleted solved parts of the conversation)

Rich client

...

  • Since the package focuses on API test I don't see as a bad thing to call API methods like err = hub.Post(api.ApplicationsRoot, &r_the_application) (feel free challenge this, it's up for discussion)

That is fine for the API test (testing the application endpoint) but would rather not have that everywhere else that needs to do to create an application for setup/tear-down. That said, I agree the rich client only insulates building the URL and can easily be convinced it's not worth it. This is mainly a response to Dylan and David wanting re-usable Fixtures.

  • The test building blocks which I'd see more practical might cover one or more API calls with raising (custom) errors as an input for assertions.

Can you elaborate?

  • Points above are partially caused by a step away from Hub core development I tried to do when started focusing on "Upstream CI" effort to get better overview on Konveyor development/QA/release process, so I'm little afraid if the Hub test suite we build could be re-used by others like QE (not meaning to be only technically possible to be re-used, but to be practical and beneficial for others to re-use it).

This seems to be an argument for the Rich Client but suspect I misunderstand.

On the Rich Client / fixtures to be reasonable building blocks for other tests - I'm going to write the code do see how it looks and works and will update here.

@djzager djzager moved this to In Progress in Konveyor Integration Mar 30, 2023
aufi added a commit that referenced this issue May 4, 2023
First part of Hub REST API test suite.

Steps
- the simplest CRUD for all resource types
- simple seeds tests
- connecting resources with refs (update tests above)
- clarify non-trivial fixtures definition and tear up&down
- basic integration scenarios (addons&analysis)

Related to #262

---------

Signed-off-by: Marek Aufart <[email protected]>
@dymurray dymurray moved this from In Progress to Done in Core May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sprint 1 testing triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: Done
Status: No status
Status: In Progress
Development

No branches or pull requests

3 participants