RDM-4217 - Create Case Data with document attachment in perftest env #16

kapiljain786 · 2019-06-11T10:12:38Z

PLEASE DO NOT MERGE THIS PR to Master.

This PR is still using Maven and Master using Gradle so I don't want to merge this PR into master.

Please just review this PR so can use this branch to complete data alignment in preftest environment to match PROD data volume.

Before creating a pull request make sure that:

commit messages are meaningful and follow good commit message guidelines
README and other documentation has been updated / added (if needed)
tests have been updated / new tests has been added (if needed)

Please remove this line and everything above and fill the following sections:

JIRA link (if applicable)

https://tools.hmcts.net/jira/browse/RDM-4217

Change description

Align perftest environment CCD data to Prod

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[ ] No

…g performance testing

S2S update (CNP)

hemantt · 2019-06-11T11:20:53Z

src/test/resources/application.conf

  createCaseUrl = "caseworkers/6687/jurisdictions/PROBATE/case-types/GrantOfRepresentation/cases"
 }

-cnp_sprod {
+cnp_sandbox {


@kapiljain786 should this be sandbox or perftest?

Agreed @hemantt. Sandbox and SPROD environment not exist any more so this configuration is not required. Removed SPROD and Sandbox config and added config for new perftest environment

hemantt · 2019-06-11T11:22:47Z

src/test/resources/data/listofcases

@@ -0,0 +1,31 @@
+id


@kapiljain786 these user-ids, isnt the file name misleading?

These are not user-id and actually document ids. however listofcases.csv file is not required for CCD performance testing so removing this file from repo.

mario-paniccia

please hold on on merging I'd like to review this properly

…into CaseDocs # Conflicts: # gradle/wrapper/gradle-wrapper.jar # gradle/wrapper/gradle-wrapper.properties # src/test/resources/application.conf # src/test/resources/logback-test.xml # src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/GetCaseData.scala # src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/GetPaginationMetaData.scala # src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/GetUserProfile.scala # src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/PostCaseData.scala # src/test/scala/uk/gov/hmcts/ccd/simulation/CCDPTSimulation.scala # src/test/scala/uk/gov/hmcts/ccd/simulation/CCDSimulation.scala # src/test/scala/uk/gov/hmcts/ccd/util/CcdTokenGenerator.scala # src/test/scala/uk/gov/hmcts/ccd/util/PerformanceTestsConfig.scala

…into CaseDocs

gbenadikar

some comments

gbenadikar · 2019-06-18T10:56:30Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/CreateDIVCaseData.scala

+import scala.util.Random
+
+
+object CreateDIVCaseDataKJ extends PerformanceTestsConfig {


Can you rename this object to CreateDIVCaseData without KJ?

This file is not needed under scenario folder so removed

gbenadikar · 2019-06-18T11:00:31Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/CreateDIVCaseData.scala

+
+ // val EventId = "applyForGrant"
+  private val rng: Random = new Random()
+  private def d8MarriagePetitionerName(): String = rng.alphanumeric.take(10).mkString


You could have created this definition probably in PerformanceTestConfig or another abstract class CreateCaseData with a generic name and used it in the scenario like
("D8MarriagePetitionerName", getRandomGeneratedString())

As we discussed about this already for each case field we need to enter data with different length so would prefer to keep it like this.

gbenadikar · 2019-06-18T12:42:58Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/CreateSSCSCaseData.scala

+import scala.util.Random
+
+
+object CreateSSCSCaseDataKJ extends PerformanceTestsConfig {


This file is not needed under scenario folder so removed

gbenadikar · 2019-06-18T12:46:27Z

...test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/CrossCaseTypeAliasFuzzySearchOnText.scala

+
+import scala.concurrent.duration._
+
+object CrossCaseTypeAliasFuzzySearchOnText extends PerformanceTestsConfig {


Can we move all ES tests to another branch? Should not be part of this PR for create case data?

Yes Moved Elastic Search, Cross Case type stuff in separate branch.

…in seperate branch

gbenadikar

Looks good only one point will like to raise is that, the create scripts, create case data in the initial state and the cases are not progressed to subsequent states. For more realistic prod like data, there should be substantial event history for the cases. Assuming this is a known fact when running the perf tests.

mario-paniccia

found multiple issues. High level of duplication. Misleading naming. New code lacks a proper structure. All this results in problems in extending and maintaining this in the future.
The comments I add are only some comments. I stopped reviewing after a while.

mario-paniccia · 2019-06-19T13:40:28Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/GetPrintableDocumentsForEvent.scala

+
+object GetPrintableDocumentsForEvent extends PerformanceTestsConfig {
+
+  private val url: String = config.getString("caseDataUrl") + "/" + config.getString("getPrintableDocumentsForEvent")


rename getPrintableDocumentsForEvent to getCasePrintableDocuments please. In other places in this file too

mario-paniccia · 2019-06-19T13:41:14Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/GetPrintableDocumentsForEvent.scala

+  def httpRequest() = {
+    val s2sToken = CcdTokenGenerator.generateGatewayS2SToken()
+    val userToken = CcdTokenGenerator.generateWebUserToken()
+    http("TX09_CCD_GetPrintableDocumentsForEvent_getDocumentsForEvent")


what's the meaning of TX09 ?

that's transaction name needed to identify timing for specific endpoint from gatling raw data

mario-paniccia · 2019-06-19T13:43:10Z

src/test/resources/listoffiles.csv

@@ -0,0 +1,2 @@
+filename


can we move these files into a subfolder tree called casesGenerator\documents

Would like to keep test related static data files under resource folder. that's fine

mario-paniccia · 2019-06-19T13:44:31Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/GetPrintableDocumentsForEvent.scala

+
+import scala.concurrent.duration._
+
+object GetPrintableDocumentsForEvent extends PerformanceTestsConfig {


rename to GetCasePrintableDocuments

mario-paniccia · 2019-06-20T07:13:07Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/SearchInputDetails.scala

+  def httpRequest() = {
+    val s2sToken = CcdTokenGenerator.generateGatewayS2SToken()
+    val userToken = CcdTokenGenerator.generateWebUserToken()
+    http("TX07_CCD_SearchInputDetails_searchInputDetails")


can we call this CCD_SearchInputDetails_searchInputDetails. what is TX07 used for?

TX01, TX02.....TXxx all are transactions name.

mario-paniccia · 2019-06-20T07:14:54Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/SearchInputDetails.scala

+  val getSearchInputDetails = scenario("Search Input Details").during(TotalRunDuration minutes) {
+    exec(
+      httpRequest()
+    )


can you format better here please

mario-paniccia · 2019-06-20T07:31:12Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/ValidateCaseDetails.scala

+  println("create case url: " + CreateCaseUrl)
+  println("create case token url: " + CreateCaseTokenUrl)
+
+  private val url: String = config.getString("caseDataUrl") + "/" + config.getString("validateCaseDetails")


remove :String
why using: config.getString("caseDataUrl") + "/"? you can use caseDataUrl(....
rename url to validateUrl

changed url to validateUrl but other syntax looks fine

mario-paniccia · 2019-06-20T07:31:51Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/ValidateCaseDetails.scala

+  println("create case token url: " + CreateCaseTokenUrl)
+
+  private val url: String = config.getString("caseDataUrl") + "/" + config.getString("validateCaseDetails")
+  println("Retrieving validateCaseDetails URL : " + url)


what does this mean? we don't retrieve urls

Amended. it's just printing URL as output.

mario-paniccia · 2019-06-20T07:37:25Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/ValidateCaseDetails.scala

+      .pause(MinThinkTime seconds, MaxThinkTime seconds)
+  }
+
+  val waitForNextIteration = pace(MinWaitForNextIteration seconds, MaxWaitForNextIteration seconds)


is this var used anywhere? can't find any usage

why we need pace?

Removed this line.

mario-paniccia · 2019-06-20T07:43:59Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/ValidateCaseDetails.scala

+
+import scala.concurrent.duration._
+
+object ValidateCaseDetails extends PerformanceTestsConfig {


All naming in this class is completely messed up. You're not Validating a case here, you are creating a case.
Class name wrong, urls names misleading
Please review carefully and fix

Reviewed and all looks OK to me
that's validateCaseDetails test and working perfectly fine.

Object name ValidateCaseDetails is also looking fine to me.

mario-paniccia · 2019-06-20T07:45:20Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/WorkBasketInputDetails.scala

+
+object WorkBasketInputDetails extends PerformanceTestsConfig {
+
+  private val url: String = config.getString("caseDataUrl") + "/" + config.getString("workbasketInputDetails")


duplicated code. Why don't you reuse use caseDataUrl( method provided by PerformanceTestsConfig?

it's not duplicate.
string concatenation to build URLs.

mario-paniccia · 2019-06-20T07:45:47Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/WorkBasketInputDetails.scala

+      .header("ServiceAuthorization", s2sToken)
+      .header("Authorization", userToken)
+      .header("Content-Type","application/json")
+      .check(status in  (200))


remove not needed spaces please

not clear about this. remove what ?

Amended println text and reformatted code.

mario-paniccia · 2019-06-20T07:46:01Z

src/test/scala/uk/gov/hmcts/ccd/corecasedata/scenarios/WorkBasketInputDetails.scala

+    exec(
+      httpRequest()
+    )
+      .pause(MinThinkTime seconds, MaxThinkTime seconds)


correct formatting please

mario-paniccia · 2019-06-20T07:55:20Z

src/test/scala/uk/gov/hmcts/ccd/data/CreateDIVCaseData.scala

+object CreateDIVCaseData extends PerformanceTestsConfig {
+
+
+  private val rng: Random = new Random()


hi level of duplication found here. Searching for 1000000 + rng.nextInt(9999999 - 1000000) + 1 finds 5 instances. This means we could introduce some helper methods
and reuse those rather than duplicating.
Even better we should have all these random values generation code extracted into a dedicated class. And reused everywhere

mario-paniccia · 2019-06-20T07:59:16Z

src/test/resources/application.conf

+  prodSSCSPagination = "caseworkers/560966/jurisdictions/SSCS/case-types/Benefit/cases/pagination_metadata?case.caseReference=test123456"
+  prodDIVORCED8caseReference = "aggregated/caseworkers/560966/jurisdictions/DIVORCE/case-types/DIVORCE/cases?page=1&case.D8caseReference=EZ12D81234"
+  prodDIVORCED8caseReferencePagination = "caseworkers/560966/jurisdictions/DIVORCE/case-types/DIVORCE/cases/pagination_metadata?case.D8caseReference=EZ12D81234"
+  createCaseDIVUrl = "caseworkers/81d4aa29-7ba2-4884-a5a4-e2a0211bfe7c/jurisdictions/:jurisdictions_reference/case-types/:casetype_reference/cases"


createCaseDIVUrl, createCaseSSCSUrl, createCaseCMCUrl... all duplicated. Needs sorting out

mario-paniccia · 2019-06-20T07:59:47Z

src/test/resources/application.conf

  ESSearch = "searchCases"
+  docStoreBashURL123 = "https://dm-store-perftest.service.core-compute-perftest.internal"


docStoreBashURL123? what does that means?

docStoreBashURL123 not needed. removed

mario-paniccia · 2019-06-20T08:01:21Z

src/test/resources/application.conf

+  docStoreBashURL123 = "https://dm-store-perftest.service.core-compute-perftest.internal"
+  docStoreBashURL = "https://gateway-ccd.perftest.platform.hmcts.net"
+  prodSSCSWorkBasket = "aggregated/caseworkers/560966/jurisdictions/SSCS/case-types/Benefit/cases?view=WORKBASKET&page=1&case.caseReference=test1234567"
+  prodSSCSPagination = "caseworkers/560966/jurisdictions/SSCS/case-types/Benefit/cases/pagination_metadata?case.caseReference=test123456"


560966 used in multiple places. Duplication problem. Perhaps extract in a variable and reuse it?

kjain added 13 commits July 26, 2018 15:53

Added extra config parameters to simualate real user conditions durin…

4bdc0bb

…g performance testing

Added extra config parameters to simualate real user conditions durin…

27ce73d

…g performance testing

Added extra config parameters to simualate real user conditions durin…

a3bd088

…g performance testing

Merge branch 'master' of github.com:hmcts/ccd-test-performance

6daf565

S2S update (CNP)

Doc Store Performance Test added

26ecfa1

Ignore mac files and results

15a4e77

added ES endpoint tests

7e87c7e

Amended ES scenarion

74f8a73

Align SPROD CCD Data

738cfc8

Adding Probate Data Injection

baaa325

adding DIV & SSCS create case data

5c83999

Adding updated config

6de74a7

RDM-4217 Create Case Data with Document attached

c21ae2f

kapiljain786 requested review from mario-paniccia and hemantt June 11, 2019 10:12

hemantt reviewed Jun 11, 2019

View reviewed changes

Removing listofcases csv and SPROD, Sandbox environment configuration

491138f

mario-paniccia reviewed Jun 13, 2019

View reviewed changes

gbenadikar added 4 commits June 17, 2019 15:14

add gradle scripts

caf42db

Merge branch 'master' of https://github.com/hmcts/ccd-test-performance …

94744e4

…into CaseDocs

Merge branch 'master' of https://github.com/hmcts/ccd-test-performance …

10bbf06

…into CaseDocs

kapiljain786 assigned kapiljain786 and gbenadikar and unassigned kapiljain786 and gbenadikar Jun 18, 2019

kapiljain786 requested a review from gbenadikar June 18, 2019 10:29

gbenadikar suggested changes Jun 18, 2019

View reviewed changes

kjain added 2 commits June 19, 2019 10:02

Addressing giri's comments

212dae4

Addressing reviewing comments from Giri - Moving Elastic Search test …

3134fb5

…in seperate branch

kjain added 3 commits June 19, 2019 11:52

Removing build folder

d007f60

Removing .gradle folder

d77d02f

Updated application.conf and removed the sensitive info

ea80273

gbenadikar approved these changes Jun 19, 2019

View reviewed changes

mario-paniccia suggested changes Jun 19, 2019

View reviewed changes

mario-paniccia reviewed Jun 19, 2019

View reviewed changes

mario-paniccia reviewed Jun 20, 2019

View reviewed changes

		import scala.util.Random


		object CreateDIVCaseDataKJ extends PerformanceTestsConfig {

		import scala.util.Random


		object CreateSSCSCaseDataKJ extends PerformanceTestsConfig {


		import scala.concurrent.duration._

		object CrossCaseTypeAliasFuzzySearchOnText extends PerformanceTestsConfig {


		object GetPrintableDocumentsForEvent extends PerformanceTestsConfig {

		private val url: String = config.getString("caseDataUrl") + "/" + config.getString("getPrintableDocumentsForEvent")


		import scala.concurrent.duration._

		object ValidateCaseDetails extends PerformanceTestsConfig {


		object WorkBasketInputDetails extends PerformanceTestsConfig {

		private val url: String = config.getString("caseDataUrl") + "/" + config.getString("workbasketInputDetails")

		object CreateDIVCaseData extends PerformanceTestsConfig {


		private val rng: Random = new Random()

		ESSearch = "searchCases"
		docStoreBashURL123 = "https://dm-store-perftest.service.core-compute-perftest.internal"

RDM-4217 - Create Case Data with document attachment in perftest env #16

Are you sure you want to change the base?

RDM-4217 - Create Case Data with document attachment in perftest env #16

Conversation

kapiljain786 commented Jun 11, 2019 • edited Loading

JIRA link (if applicable)

Change description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mario-paniccia left a comment

Choose a reason for hiding this comment

gbenadikar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbenadikar left a comment • edited Loading

Choose a reason for hiding this comment

mario-paniccia left a comment • edited Loading

Choose a reason for hiding this comment

mario-paniccia Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mario-paniccia Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kapiljain786 commented Jun 11, 2019 •

edited

Loading

gbenadikar left a comment •

edited

Loading

mario-paniccia left a comment •

edited

Loading

mario-paniccia Jun 19, 2019 •

edited

Loading

mario-paniccia Jun 19, 2019 •

edited

Loading