Read suffix or prefix from file

[S-Koell]

In order to have deviating pre-/suffixes than the default ones in the guidelines, it would be great to have them optionally defined externally.

At the moment they are hardcoded into the validator.

Requirements:

• This change shall not infringe with the current behaviour. Therefore if no external definition is present the validator uses the default (hardcoded) ones.
• If NO external configuration is present, the tool should NOT communicate that
• This feature should work with the command line and the sql developer plugin and regardless of environment (therefore a realtive path should probably be avoided. E.g. user home dir)
• Test cases should be present to validate that it still works with or without the external configuration present

( see pull request: #12 )

[PhilippSalvisberg]

Perfect. Thanks @S-Koell . I suggest to use something like this for the property file:

File propertyFile = new File(System.getProperty("user.home") + File.separator + "TrivadisGuidelines3Plus.properties");

This should work for all operating systems. Naming the property file according the validator class should make the relation and its purpose clear.

[S-Koell]

Okay I'll implement it like that :).
Do you have tips on how to write the testcases? How to make a testcase with and one without the .properties file present if it is looking at the user dir?

[PhilippSalvisberg]

You could handle that in TrivadisGuidelines3PlusTest.xtend.

I'd add a setup method (annotated with @BeforeClass) deleting the property file, if it exists. This should ensure that all existing test cases work, since they are based on the hard coded prefixes and suffixes.

Then I'd add an additional test case, creating a property file with a few properties different to the defaults and test, if they are applied. That's it.

[PhilippSalvisberg]

And don't forget the cleanup. You may annotate the setup method with @AfterClass as well. That's fine with the delete approach. But maybe some more clever mechanism could ensure that an existing property file will not be lost due to a test run. E.g. by renaming instead of deleting it. Of course it should work also if there is nothing to rename.

[S-Koell]

I would still leave an example properties in this repo under (a newly created) /resources folder instead of the plugin folder.
Are you okay with that or would you prefer to leave it out completly?

[PhilippSalvisberg]

No, I'm against it. The reason is that I suspect that I need two property files when I use the CLI and the SQL Developer extension on one computer. I think that would be wrong.

[S-Koell]

But the custom properties are gonna be in the user home folder? The ones from this repo are just for guidance and will not be used from the /resources folder.
If I leave the properties out of this repo I would add the contents (defaults) to the readme.

A question regarding the tests:
We have problems testing it because JUnit has no ordering of tests. All tests may run in a random order.
If we implement our test cases in the TrivadisGuidelines3PlusTest.xtend class we have difficulties with the creation/deletion of the properties.

We have 3 possible solutions:

• Upgrade to JUnit 5, where we can make a subclass for the property tests (@nested annotation)
• Use a seperate test class for the testing of the properties
• Use @FixMethodOrder annotation so that the order of the tests is fixed (but depends on the naming). In my opinion the worst solution.

[PhilippSalvisberg]

You meant an example properties file. Still, I do not think it is necessary. It's clear based on the code and the test case.

Regarding the tests I see another solution. The class specific setup ensures that no property file is available. Either via rename or delete. This ensures that all existing test cases work as today. For test cases requiring a property file, do the setup as first thing in the test case (create the file with the relevant properties) and do the cleanup (delete this property file) at the end of the test case. This should do the trick.

[S-Koell]

Okay.

Regarding testing:
That doesnt work because the class (TrivadisGuidelines3Plus) is instantiated before the body of each @test method runs.
Only solution left is to call the method in the TrivadisGuidelines3Plus which reads the properties (again):

        @Test 
	def void propertieTest() {
		createProperties()
                TrivadisGuidelines3Plus.readProperties()
                //Test ...
	}

But I dont know how to access the TrivadisGuidelines3Plus instance yet because we have no variable.

[PhilippSalvisberg]

Look at setupValidator. This is initiated before every test. At this point you can delete/rename the existing property file. You could extend this method. I would add an additional one. Order does not matter in this case. This should really work. If it doesn't, then I suggest that you commit the test class in the PR and I can point out the changes based on the code to make it work.

[S-Koell]

setupValidator has @BeforeClass.
That means it is only executed once.

[S-Koell]

The problem is not the deletion/movement. The problem is the creation!
I can't create the properties in the @test because the class already ran the constructor at that time (which means no Properties loaded).
A method before every test wouldn't help either because the existing tests need to be testet without the properties.

[PhilippSalvisberg]

Of course. You are right. I did not see the problem. Sorry.

Ok, in this case I suggest the following:

a) extend the existing test class TrivadisGuidelines3PlusTest

• setupValidator to rename existing property
• cleanupValidator to undo the previous rename (if there was a file)

b) create a new test class TrivadisGuidelines3PlusPropertyFileTest

• setupValidator to rename existing property
• cleanupValidator to undo the previous rename (if there was a file)
• add some test methods