lczub / TestLink-API-Python-client

A Python client to use the TestLink API
105 stars 63 forks source link

Add testreporter, _setParams and getTestCaseByVersion #94

Closed Brian-Williams closed 6 years ago

Brian-Williams commented 6 years ago
  1. TestReporter's subclasses can be used to idempotently generate required components of testlink before sending the report to a specified testcase.
  2. TestGenReporter is a default combination of all the TestReporter's subclasses and will try to generate everything it's subclasses is capable of.
  3. Added TestLinkHelper._setParams to simplify libraries that need to overwrite how a helper aquires it's parameters
  4. Added TestlinkAPIClient.getTestCaseByVersion to expose the common need of gettting the latest testcase by default. It was already used, but not a function in this class and has uses elsewhere.
Brian-Williams commented 6 years ago

Hi lczub/Luiko,

a) TestGenReporter doesn't have hard coded url or dev key. It's goal is to present the most common use case of always reporting a result if at all possible. In my mind it's value is if other fields are generatable in the future someone's code wouldn't have to be changed in order to get those new benefits. They would simply update this dependency. This model is sometimes used in security repos where there are default objects that will update with the spec. I believe passlib does this.

b) I've moved and provided better docs for TestGenReporter. Not importing the functional classes from TestReport is intentional. They started as and are still mostly intended as Mixin classes. I never intended for them to be used in isolation, but after writing them realized some organizations might want only that one functionality. It would be a little silly to force them to create a new mixin instead of importing the functional one. i.e. why force MyTestReporter = TestReporter(AddTestCaseReporter) when they can simply import the class that adds that functionality AddTestCaseReporter.

However, I still think that is an edge case. The vast majority of users will be using the classes effectively as mixins which are only adding functionality, which is why the classes have function/method style names. I understand not everyone likes mixin design or naming functions so am open to change, but that was my reasoning.

c) That's a better name, done. The naming style is explained above in b.

c2) I don't believe it does. Is this talking about the docstring? In my experience most teams using this will have an automation dev key and a single testlink server they are setting it up with. If that's not your experience I would be open to changing the docstring.

d+e) I can try to do this when I have more time.

f) Changed to TLArgError.

I am unexpectedly busy all of the sudden. I will try to get this fit for purpose, but if my responses are slow I apologize in advance.

Thanks, Brian

lczub commented 6 years ago

Hello Brian-Williams,

I very much apologize for my missreading of your code. It seams, that I had eggs on my eyes, cause I always read the last lines of TestGenReport class comment as functional code and does not recognize, that this lines are a sample inside the class comment. Shame on me. I now understand your intention to import only TestGenReport and also understand your time pressure.

I think with your separation of TestGenReport this pull request is ok and I will pull it into the master and integrate it later on into the tl-future branch. Test and usage.rst will be separate tasks, if we find time for them.

Many thanks for your work Regards Luiko

lczub commented 6 years ago

Hello Brian-Williams, fyi, with commit 1a8b5d8 I extended the examples for your pull request and with #95 I defined a change request, how the status reporting should be done more flexible. I will try to find a solution for that in the next weeks.

Regards Luiko