Week3 java consolidate test code

[tringuyen-yw]

• Reduce code redundancy
• Move Astra connection info to src/main/resources/application.conf
• Add assertions
• Fix logback-test.xml now log from ALL tests are visible
• Test Ex05_Query5c_Travel is now idempotent (each run only add 50 new sensors reading, previous reading are deleted)

[clun]

Thank you for the PR, always appreciated.

• AstraConnectionInfo: Even I totally agree with dedicated bean or properties injected instead of static constants you need to know that most of people are total beginner and we got very basic questions, idea was to have same code as Python and Node.

• Landing and takeoff really don't need the journey Summary as this is only when creating Journey I would rather not touching them (align with other code is a strong part here)

• methods : Delete, switching parameters ok why not, externalization of statement in utility methods, I like that it does not make the code more complex I guess

• Fixture: Ok that's a good idea but please change the READ_JAVA also then to reflect your changes people do copy paste :)

• Testutils and properties : Simply update the application.conf with relevant values and the connection will created https://docs.datastax.com/en/developer/java-driver/4.6/manual/core/configuration/ no need for properties :) Again here idea is to align with other language.

Could you please update the PR accordingly to merge. I really appreciate all efforts you put in there.

[tringuyen-yw]

@clun Thanks for the thorough review.

Landing and takeoff really don't need the journey Summary as this is only when creating Journey I would rather not touching them (align with other code is a strong part here)

This is true. The reason I introduce this summary update is because the Ex03_Query5a_Insert_Journey.java is not idempotent. Each time it runs, a new journey is created with an auto-generated journey_id by JourneyRepository.create(). However in all subsequent tests, a constant journey_id is used 8dfd0a30-c73b-11ea-b87b-1325d5aaa06b. As a result, takeoff and landing tests upsert a different row than the one created by the Insert_Journey test, and the summary is always null.

The developer must pay attention when verifying the data by Astra CQL Console.
I found that using the summary is a simple trick to make the verification simpler. Even though you can still verify by looking at columns journey_id, active, start, end in spacecraft_journey_catalog table.

Ultimately, I agree that there is no need to update the summary. However, I think the consistency of the test logic is a bigger concern than the update detail in question here.

[tringuyen-yw]

@clun

Fixture: Ok that's a good idea but please change the READ_JAVA also then to reflect your changes people do copy paste :)

Can you please elaborate on READ_JAVA ? Link, java class, line, number, etc. and give some guidance?

[tringuyen-yw]

methods : Delete, switching parameters ok why not, externalization of statement in utility methods, I like that it does not make the code more complex I guess

switching parameters position: this is probably unnecessary indeed. It just that the partition key is (spacecraft_name, journey_id) and somehow it works better in my mind when the parameters are laid out in the same sequence.

externalization of statement, this whole thing was to avoid code repetition b/c I added JourneyRepository.deleteSensorReadings() to make the Ex05_Query5c_Travel test idempotent. Try to run this test now, you will not know if it was running OK (becase logback-test.xml doesn't enable log for this class). Then you will end up running this test many times just to figure out what is going on. Once you figure out, you realize that each run inserts 50 records. And the SELECT you verify in CQL Console sho many hundreds of rows.

[tringuyen-yw]

Testutils and properties : Simply update the application.conf with relevant values and the connection will created https://docs.datastax.com/en/developer/java-driver/4.6/manual/core/configuration/ no need for properties :) Again here idea is to align with other language.

Oh this is good thanks for the help! I like HOCON config format. But was concerned of adding additional dependencies. I saw the application.conf file but couldn't figure out how to read from it (Java is not my main programming language). Definitely, this is way better than the properties file.

[tringuyen-yw]

@clun Can you please review again. Changes:

• Landing and takeoff no longer update the Journey summary (ie. revert changes in landing and takeoff in JourneyRepository)
• Astra Connection config moved to application.conf using the typesafe.config framework as you suggested
• Add assertions to takeoff and landing test

Actually we can add assertions in every test (as you suggested suing the live workshop). But I'll stop there. A motivated student could follow the pattern and add the missing assertions.

[clun]

Thank you for this updates. Will execute the workshop with this code, eventually update the read me accordingly an MERGE !