Enhance string concatenations for url and file path

[danghai]

I make PR to enhance string concatenations for url and file path Fixed #77

[coveralls]

Pull Request Test Coverage Report for Build 372

• 7 of 16 (43.75%) changed or added relevant lines in 3 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.4%) to 28.767%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
sktm/jenkins.py	0	1	0.0%
sktm/patchwork.py	1	9	11.11%
<!--	Total:	7	16	43.75%	-->

Files with Coverage Reduction	New Missed Lines	%
sktm/patchwork.py	2	16.84%
<!--	Total:	2		-->

Totals
Change from base Build 367:	0.4%
Covered Lines:	319
Relevant Lines:	932

---

💛 - Coveralls

[danghai]

@veruu Thanks feedback. I fixed it, and also add the test cases for it.

[veruu]

@danghai good idea with adding the tests! However, I still don't think we should be removing the trailing slash if it was specified. The point of the function is to be able to substitute it for every join or format in sktm (so the xmlrpc on shouldn't be skipped), and if that requires trailing slash, it shouldn't be removed. Also, because of this, it would be great if the function was moved to __init__ (or new utils.py), instead of being imported from patchwork.py elsewhere.

Does it make sense? What do you think?

[danghai]

@veruu you mean only delete the leading slash for suffix? I agree with moving this function in __init__. That makes sense. Could you review it know?

[veruu]

Looks better now! can you also add a test that would ensure a trailing slash in the suffix stays, and move the rest of joins/format strings in the repo to use this method, so all related changes are in a single commit?

[danghai]

@veruu I fix it. I encounter a hard one.

        return "%s/job/%s/%s" % (self.server.base_server_url(), jobname,
                                 buildid)

It is really confused when we add this function in here because we need to call it 3 times.

[veruu]

Agreed, that one isn't exactly nice, and we shouldn't convert it in the current state. I'm thinking, eventually, it would be nice to have the function accept a base and a list of suffixes. It will need some more thoughts about the implementation to have it work properly though, so I don't want to mix it in here, just a food for thought for later :)

[danghai]

@veruu Right, it is better to have a base and a list of suffixes. What about introducing *suffix as a list of suffixes?

[danghai]

@veruu I fix it and add docstring for this function. Could you review now? I do not see any os.path.join, .format in sktm repo. Or you think skt repo?

[veruu]

@danghai there are few os.path.joins in the patchwork.py file. You can find them by running git grep "os.path.join". Some of them have a related comment which can be removed as they are exchanged too :)

[danghai]

@veruu I fixed it.

[danghai]

@veruu Thanks. Do you want to use this function for skt too without test cases?

[veruu]

@danghai sure, adding it to skt would be great! However do copy the test cases over as well please, since the implementations will be separate and it's possible one of them will have a bug introduced