search

amino-os / Amino.Run

Amino Distributed OS - Runtime Manager
Apache License 2.0
29 stars 12 forks source link

Replace unnecessary DM name strings with classes in MultiDMTestCases #804

Closed quinton-hoole closed 5 years ago

quinton-hoole commented 5 years ago

This is part of an attempt to make multiDM integration tests sane, by autogenerating an integration test per combination of DM's.

It is based on #803, so only the third commit 1de958c and onward need to be reviewed. The first two commits will disappear once #803 is merged and rebased against.

All tests succeed: 

./gradlew check
BUILD SUCCESSFUL in 1m 22s
55 actionable tasks: 13 executed, 42 up-to-date
quinton-hoole commented 5 years ago

Yes, but not worth lots of messy code.

On Tue, Jun 25, 2019, 22:39 Amit Roushan notifications@github.com wrote:

@AmitRoushan commented on this pull request.

In core/src/integrationTest/java/amino/run/multidm/MultiDMTestCases.java https://github.com/amino-os/Amino.Run/pull/804#discussion_r297490300:

     MicroServiceSpec spec =

MicroServiceSpec.newBuilder()

  • .setName(testName)

IMHO, It is good to have micro service application deployed with different names. I may help in reusing same testcases for running in parallel.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/amino-os/Amino.Run/pull/804?email_source=notifications&email_token=AKNAA6CSJKGXZSEE6ZZX7H3P4L6K3A5CNFSM4H3NL7K2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB4U23EI#pullrequestreview-254389649, or mute the thread https://github.com/notifications/unsubscribe-auth/AKNAA6G3J3LVJJDVBEMGVULP4L6K3ANCNFSM4H3NL7KQ .

AmitRoushan commented 5 years ago

@quinton-hoole May be we can add one more argument in runTest (String testcaseName, Class... dmClasses) and use it for setting microservice name.

quinton-hoole commented 5 years ago

It's not actually a test name. That variable name is inaccurate. That name is the name of a microservice. Which is itself not used anywhere. This code is all rubbish. To populate that name it would need to be generated somewhere. I don't think that the name is used anywhere. Unless there is a concrete reason to add the somewhat complex code to generate a unique name for a field that doesn't make sense and is not used anywhere, I think we just remove all the confusing rubbish, as I have tried to do.

quinton-hoole commented 5 years ago

Addressed review feedback. Rebased. Merging.