Skip to content

Update integration tests to run as upgrade tests for multitenancy #8799

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

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

jbhamra1
Copy link
Contributor

@jbhamra1 jbhamra1 commented Apr 20, 2023

we now start using dgraphtest package instead of testutil package. This PR also modifies the tests in systest/multi-tenancy package to run as both integration as well as upgrade tests.

The below test functions in systest/multi-tenancy/basic_test.go file are modified to run for 'upgrade' and 'integration' test using dgraphtest framework. For 'integration', Upgrade() is a NOOP.

func (suite *MultitenancyTestSuite) TestAclBasic() {
func (suite *MultitenancyTestSuite) TestNameSpaceLimitFlag() {
func (suite *MultitenancyTestSuite) TestPersistentQuery() {
func (suite *MultitenancyTestSuite) TestTokenExpired() {
func (suite *MultitenancyTestSuite) TestTwoPermissionSetsInNameSpacesWithAcl() {
func (suite *MultitenancyTestSuite) TestCreateNamespace() {
func (suite *MultitenancyTestSuite) TestResetPassword() {
func (suite *MultitenancyTestSuite) TestDeleteNamespace() {

systest/multi-tenancy/integration_test.go and systest/multi-tenancy/upgrade_test.go contains the testify/suite functions and datatypes to start the test.

integration_basic_helper_test.go contain test functions that are run for integration test.

@ghost ghost added area/testing Testing related issues go Pull requests that update Go code labels Apr 20, 2023
Copy link
Member

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

still reviewing

@jbhamra1 jbhamra1 force-pushed the jassi/upgrade-multi-tenancy branch from df25dc7 to a19f192 Compare May 11, 2023 14:57
@jbhamra1 jbhamra1 force-pushed the jassi/upgrade-multi-tenancy branch from a19f192 to 2776ed8 Compare May 17, 2023 07:09
Copy link
Member

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

The PR looks much cleaner, thanks for all the work.

@jbhamra1 jbhamra1 force-pushed the jassi/upgrade-multi-tenancy branch from 2776ed8 to 59d1575 Compare May 18, 2023 15:00
@mangalaman93 mangalaman93 changed the title Multitenancy test, draft 1 Upgrade testing for multitenancy May 22, 2023
@mangalaman93 mangalaman93 force-pushed the jassi/upgrade-multi-tenancy branch from 59d1575 to b0600b6 Compare May 22, 2023 23:50
@mangalaman93 mangalaman93 changed the title Upgrade testing for multitenancy Update integration tests to run as upgrade tests for multitenancy May 23, 2023
@mangalaman93 mangalaman93 force-pushed the jassi/upgrade-multi-tenancy branch from b0600b6 to 172828e Compare May 23, 2023 07:04
@mangalaman93 mangalaman93 marked this pull request as ready for review May 23, 2023 07:04
@jbhamra1 jbhamra1 force-pushed the jassi/upgrade-multi-tenancy branch from 172828e to badbf40 Compare May 25, 2023 10:20
@mangalaman93 mangalaman93 force-pushed the jassi/upgrade-multi-tenancy branch 2 times, most recently from ce9c16e to b6ee096 Compare May 26, 2023 04:58
mangalaman93
mangalaman93 previously approved these changes May 26, 2023
@mangalaman93 mangalaman93 force-pushed the jassi/upgrade-multi-tenancy branch from b6ee096 to f13e02d Compare May 26, 2023 05:02
@all-seeing-code
Copy link
Contributor

we now start using dgraphtest package instead of testutil package. This PR also modifies the tests in systest/multi-tenancy package to run as both integration as well as upgrade tests.

@jbhamra1 Could you please add which tests were added and if there were test cases which were modified. A small summary would help me in reviewing the code.

@jbhamra1
Copy link
Contributor Author

we now start using dgraphtest package instead of testutil package. This PR also modifies the tests in systest/multi-tenancy package to run as both integration as well as upgrade tests.

@jbhamra1 Could you please add which tests were added and if there were test cases which were modified. A small summary would help me in reviewing the code.

The below test functions in systest/multi-tenancy/basic_test.go file are modified to run for 'upgrade' and 'integration' using dgraphtest framework. For 'integration', Upgrade() is a NOOP.

func (suite *MultitenancyTestSuite) TestAclBasic() {
func (suite *MultitenancyTestSuite) TestNameSpaceLimitFlag() {
func (suite *MultitenancyTestSuite) TestPersistentQuery() {
func (suite *MultitenancyTestSuite) TestTokenExpired() {
func (suite *MultitenancyTestSuite) TestTwoPermissionSetsInNameSpacesWithAcl() {
func (suite *MultitenancyTestSuite) TestCreateNamespace() {
func (suite *MultitenancyTestSuite) TestResetPassword() {
func (suite *MultitenancyTestSuite) TestDeleteNamespace() {

integration_test.go and upgrade_test.go contains the testify/suite functions and datatypes to start the test.

integration_basic_helper_test.go contain test functions that are run for integration test.

Copy link
Contributor

@harshil-goel harshil-goel left a comment

Choose a reason for hiding this comment

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

Great work, a bit of nitpicks here and there.

name
}
// Upgrade
suite.Upgrade()
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel a bit uneasy calling upgrade here, given that this would be used for integration tests also. Can we improve this?

Copy link
Member

Choose a reason for hiding this comment

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

What doesn't feel right to you? For integration tests, this function is empty. It looks odd I agree, we couldn't think of a better way though.

mangalaman93
mangalaman93 previously approved these changes May 31, 2023
Copy link
Contributor

@joshua-goldstein joshua-goldstein left a comment

Choose a reason for hiding this comment

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

Putting this block here for now, because we need to fix the infrastructure in dgraphtest before adding more tests. @mangalaman93

@mangalaman93 mangalaman93 force-pushed the jassi/upgrade-multi-tenancy branch from d3ee584 to f248367 Compare June 3, 2023 15:23
mangalaman93
mangalaman93 previously approved these changes Jun 3, 2023
mangalaman93
mangalaman93 previously approved these changes Jun 3, 2023
@mangalaman93 mangalaman93 force-pushed the jassi/upgrade-multi-tenancy branch from 57f7233 to 2b0a742 Compare June 3, 2023 18:53
mangalaman93
mangalaman93 previously approved these changes Jun 3, 2023
@mangalaman93 mangalaman93 force-pushed the jassi/upgrade-multi-tenancy branch from 2b0a742 to 9af1765 Compare June 4, 2023 03:36
@mangalaman93 mangalaman93 changed the base branch from main to aman/upgrade-testing June 5, 2023 17:28
@mangalaman93 mangalaman93 merged commit 8ff718a into aman/upgrade-testing Jun 5, 2023
@mangalaman93 mangalaman93 deleted the jassi/upgrade-multi-tenancy branch June 5, 2023 17:28
mangalaman93 pushed a commit that referenced this pull request Jun 5, 2023
)

we now start using dgraphtest package instead of testutil package. This
PR also modifies the tests in systest/multi-tenancy package to run as
both integration as well as upgrade tests.

The below test functions in systest/multi-tenancy/basic_test.go file are
modified to run for 'upgrade' and 'integration' test using dgraphtest
framework. For 'integration', Upgrade() is a NOOP.

func (suite *MultitenancyTestSuite) TestAclBasic() {
func (suite *MultitenancyTestSuite) TestNameSpaceLimitFlag() {
func (suite *MultitenancyTestSuite) TestPersistentQuery() {
func (suite *MultitenancyTestSuite) TestTokenExpired() {
func (suite *MultitenancyTestSuite)
TestTwoPermissionSetsInNameSpacesWithAcl() {
func (suite *MultitenancyTestSuite) TestCreateNamespace() {
func (suite *MultitenancyTestSuite) TestResetPassword() {
func (suite *MultitenancyTestSuite) TestDeleteNamespace() {

systest/multi-tenancy/integration_test.go and
systest/multi-tenancy/upgrade_test.go contains the testify/suite
functions and datatypes to start the test.

integration_basic_helper_test.go contain test functions that are run for
integration test.
mangalaman93 pushed a commit that referenced this pull request Jun 6, 2023
)

we now start using dgraphtest package instead of testutil package. This
PR also modifies the tests in systest/multi-tenancy package to run as
both integration as well as upgrade tests.

The below test functions in systest/multi-tenancy/basic_test.go file are
modified to run for 'upgrade' and 'integration' test using dgraphtest
framework. For 'integration', Upgrade() is a NOOP.

func (suite *MultitenancyTestSuite) TestAclBasic() {
func (suite *MultitenancyTestSuite) TestNameSpaceLimitFlag() {
func (suite *MultitenancyTestSuite) TestPersistentQuery() {
func (suite *MultitenancyTestSuite) TestTokenExpired() {
func (suite *MultitenancyTestSuite)
TestTwoPermissionSetsInNameSpacesWithAcl() {
func (suite *MultitenancyTestSuite) TestCreateNamespace() {
func (suite *MultitenancyTestSuite) TestResetPassword() {
func (suite *MultitenancyTestSuite) TestDeleteNamespace() {

systest/multi-tenancy/integration_test.go and
systest/multi-tenancy/upgrade_test.go contains the testify/suite
functions and datatypes to start the test.

integration_basic_helper_test.go contain test functions that are run for
integration test.
mangalaman93 pushed a commit that referenced this pull request Jun 6, 2023
)

we now start using dgraphtest package instead of testutil package. This
PR also modifies the tests in systest/multi-tenancy package to run as
both integration as well as upgrade tests.

The below test functions in systest/multi-tenancy/basic_test.go file are
modified to run for 'upgrade' and 'integration' test using dgraphtest
framework. For 'integration', Upgrade() is a NOOP.

func (suite *MultitenancyTestSuite) TestAclBasic() {
func (suite *MultitenancyTestSuite) TestNameSpaceLimitFlag() {
func (suite *MultitenancyTestSuite) TestPersistentQuery() {
func (suite *MultitenancyTestSuite) TestTokenExpired() {
func (suite *MultitenancyTestSuite)
TestTwoPermissionSetsInNameSpacesWithAcl() {
func (suite *MultitenancyTestSuite) TestCreateNamespace() {
func (suite *MultitenancyTestSuite) TestResetPassword() {
func (suite *MultitenancyTestSuite) TestDeleteNamespace() {

systest/multi-tenancy/integration_test.go and
systest/multi-tenancy/upgrade_test.go contains the testify/suite
functions and datatypes to start the test.

integration_basic_helper_test.go contain test functions that are run for
integration test.
mangalaman93 added a commit that referenced this pull request Jun 6, 2023
) (#8854)

we now start using dgraphtest package instead of testutil package. This
PR also modifies the tests in systest/multi-tenancy package to run as
both integration as well as upgrade tests.

The below test functions in systest/multi-tenancy/basic_test.go file are
modified to run for 'upgrade' and 'integration' test using dgraphtest
framework. For 'integration', Upgrade() is a NOOP.

func (suite *MultitenancyTestSuite) TestAclBasic() { func (suite
*MultitenancyTestSuite) TestNameSpaceLimitFlag() { func (suite
*MultitenancyTestSuite) TestPersistentQuery() { func (suite
*MultitenancyTestSuite) TestTokenExpired() { func (suite
*MultitenancyTestSuite)
TestTwoPermissionSetsInNameSpacesWithAcl() {
func (suite *MultitenancyTestSuite) TestCreateNamespace() { func (suite
*MultitenancyTestSuite) TestResetPassword() { func (suite
*MultitenancyTestSuite) TestDeleteNamespace() {

systest/multi-tenancy/integration_test.go and
systest/multi-tenancy/upgrade_test.go contains the testify/suite
functions and datatypes to start the test.

integration_basic_helper_test.go contain test functions that are run for
integration test.

Co-authored-by: Jassi <jbhamra@dgraph.io>
ghost pushed a commit that referenced this pull request Jul 7, 2023
) (#8854)

we now start using dgraphtest package instead of testutil package. This
PR also modifies the tests in systest/multi-tenancy package to run as
both integration as well as upgrade tests.

The below test functions in systest/multi-tenancy/basic_test.go file are
modified to run for 'upgrade' and 'integration' test using dgraphtest
framework. For 'integration', Upgrade() is a NOOP.

func (suite *MultitenancyTestSuite) TestAclBasic() { func (suite
*MultitenancyTestSuite) TestNameSpaceLimitFlag() { func (suite
*MultitenancyTestSuite) TestPersistentQuery() { func (suite
*MultitenancyTestSuite) TestTokenExpired() { func (suite
*MultitenancyTestSuite)
TestTwoPermissionSetsInNameSpacesWithAcl() {
func (suite *MultitenancyTestSuite) TestCreateNamespace() { func (suite
*MultitenancyTestSuite) TestResetPassword() { func (suite
*MultitenancyTestSuite) TestDeleteNamespace() {

systest/multi-tenancy/integration_test.go and
systest/multi-tenancy/upgrade_test.go contains the testify/suite
functions and datatypes to start the test.

integration_basic_helper_test.go contain test functions that are run for
integration test.

Co-authored-by: Jassi <jbhamra@dgraph.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Testing related issues go Pull requests that update Go code
Development

Successfully merging this pull request may close these issues.

5 participants