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.

Sign up for GitHub

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

Fix flaky recipe parameter test by removing constant resource name #4995

Merged
merged 6 commits into from
Jan 23, 2023
Merged

Fix flaky recipe parameter test by removing constant resource name #4995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7847615
fix flaky recipe parameter test by removing constant resource name
mishrapratikshya Jan 21, 2023
6c9b0f3
temp:Add log line to check the failure
mishrapratikshya Jan 21, 2023
189be40
removed debug logs
mishrapratikshya Jan 21, 2023
8ba279f
Merge branch 'main' into pratmishra/fixFlakyRecipeParameterTest
mishrapratikshya Jan 21, 2023
bcd1ba8
add the skip message
mishrapratikshya Jan 21, 2023
147d858
FIX: The name of account should be less than 44
mishrapratikshya Jan 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ stages:
INTEGRATION_TEST_SP_APP_ID: $(INTEGRATION_TEST_SP_APP_ID)
INTEGRATION_TEST_TENANT_ID: $(INTEGRATION_TEST_TENANT_ID)
INTEGRATION_TEST_SP_PASSWORD: $(INTEGRATION_TEST_SP_PASSWORD)
INTEGRATION_TEST_RESOURCE_GROUP_NAME: $(INTEGRATION_TEST_RESOURCE_GROUP_NAME)
AWS_ACCESS_KEY_ID: $(AWS_ACCESS_KEY_ID)
AWS_SECRET_ACCESS_KEY: $(AWS_SECRET_ACCESS_KEY)
AWS_REGION: $(AWS_REGION)
Expand Down
17 changes: 11 additions & 6 deletions test/functional/corerp/resources/mongodb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package resource_test

import (
"os"
"testing"

"github.com/project-radius/radius/pkg/resourcemodel"
Expand Down Expand Up @@ -219,8 +220,12 @@ func Test_MongoDB_Recipe_Parameters(t *testing.T) {
template := "testdata/corerp-resources-mongodb-recipe-parameters.bicep"
name := "corerp-resources-mongodb-recipe-parameters"
appNamespace := "corerp-resources-mongodb-recipe-param-app"

t.Skip("This test is flaky, see issue: https://github.com/project-radius/radius/issues/4992")
rg := os.Getenv("INTEGRATION_TEST_RESOURCE_GROUP_NAME")
// skip the test if INTEGRATION_TEST_RESOURCE_GROUP_NAME is not set
// for running locally set the INTEGRATION_TEST_RESOURCE_GROUP_NAME with the test resourceGroup
if rg == "" {
t.Skip("This test needs the env variable INTEGRATION_TEST_RESOURCE_GROUP_NAME to be set")
}
Comment on lines +224 to +228
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we need to add this check? How would we know if the test is just silently being skipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this check for local testing. If some one runs the test without setting INTEGRATION_TEST_RESOURCE_GROUP_NAME then it will skip.


test := corerp.NewCoreRPTest(t, name, []corerp.TestStep{
{
Expand All @@ -237,7 +242,7 @@ func Test_MongoDB_Recipe_Parameters(t *testing.T) {
App: name,
},
{
Name: "mcp-app-ctnr",
Name: "mdb-app-ctnr",
Type: validation.ContainersResource,
App: name,
},
Expand All @@ -249,12 +254,12 @@ func Test_MongoDB_Recipe_Parameters(t *testing.T) {
{
Provider: resourcemodel.ProviderAzure,
LocalID: outputresource.LocalIDAzureCosmosAccount,
Identity: "account-developer-parameters",
Identity: "acnt-developer-" + rg,
},
{
Provider: resourcemodel.ProviderAzure,
LocalID: outputresource.LocalIDAzureCosmosDBMongo,
Identity: "mongodb-developer-parameters",
Identity: "mdb-developer-" + rg,
Comment on lines +257 to +262
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these name validations needed for this particular test? Is there anything special being done on the server side for this scenario that we are trying to validate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mongo resources in the recipe are getting deployed with these names. This is what the developer sets while creating mongo link.

},
},
},
Expand All @@ -263,7 +268,7 @@ func Test_MongoDB_Recipe_Parameters(t *testing.T) {
K8sObjects: &validation.K8sObjectSet{
Namespaces: map[string][]validation.K8sObject{
appNamespace: {
validation.NewK8sPodForResource(name, "mcp-app-ctnr"),
validation.NewK8sPodForResource(name, "mdb-app-ctnr"),
},
},
},
Expand Down
6 changes: 3 additions & 3 deletions test/functional/corerp/resources/testdata/corerp-resources-mongodb-recipe-parameters.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ resource app 'Applications.Core/applications@2022-03-15-privatepreview' = {
}

resource webapp 'Applications.Core/containers@2022-03-15-privatepreview' = {
name: 'mcp-app-ctnr'
name: 'mdb-app-ctnr'
location: 'global'
properties: {
application: app.id
Expand Down Expand Up @@ -77,8 +77,8 @@ resource recipedb 'Applications.Link/mongoDatabases@2022-03-15-privatepreview' =
recipe: {
name: 'mongodb'
parameters: {
documentdbName: 'account-developer-parameters'
mongodbName: 'mongodb-developer-parameters'
documentdbName: 'acnt-developer-${rg}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a dynamically generated name for account to avoid name collisions in tests? You can either validate by matching prefix or remove account name from parameters and rely just on the database name.

mongodbName: 'mdb-developer-${rg}'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed it as the length of the account should be less than 3 characters or more than 44 characters long.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to use env variable instead of resourceGroup() function?

Copy link
Contributor Author

@mishrapratikshya mishrapratikshya Jan 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

it using the resourceGroup() function. Just created a param rg for it in the bicep.
https://github.com/project-radius/radius/blob/147d858ba2dbe8d045b0c5438e6c2500da72608c/test/functional/corerp/resources/testdata/corerp-resources-mongodb-recipe-parameters.bicep#L3

}
}
}
Expand Down