search

Snooz82 / robotframework-datadriver

Library to provide Data-Driven testing with CSV tables to Robot Framework
Apache License 2.0
131 stars 37 forks source link

Robot 6.1 compatibility #88

Closed pianofab closed 1 year ago

pianofab commented 1 year ago

I am getting the following error when attempting to upgrade to Robot 6.1 (note: this is under Python 3.10):

[ ERROR ] In source file:
  File ".xlsx", line 0
[ ERROR ] Calling method '_start_suite' of listener 'DataDriver' failed: AttributeError: 'PosixPath' object has no attribute 'rfind'
pianofab commented 1 year ago

I believe this because I had to update my own listener to work under Robot 6.1:

https://github.com/robotframework/robotframework/issues/4786

pekkaklarck commented 1 year ago

Most likely caused by this change: https://github.com/robotframework/robotframework/issues/4596

If I'm correct, fix will be easy. Unfortunate that noboby noticed this earlier.

Snooz82 commented 1 year ago

@pekkaklarck will you revert the usage of Path? i can fix this in DataDriver and maybe more other listeners as well. Actually the implementation in DataDriver is pretty poor and i am refactoring all that to Path as well.

However, i would consider this change from string to Path as a highly backwards incompatible change. I would question the balance between added value vs. caused issues.

i would bet that many other listeners do rely on this being a string.

Ps: shame on me, not testing this before the Release. 🫣

pekkaklarck commented 1 year ago

I don't think reverting the change is a good idea. The reason it was done, as explained in the issue, is that

Big reason for using Paths in parsing was making the new external parsing API to accept Path. As mentioned above, it's much more convenient than accepting str.

The change was obviously backwards incompatible and labeled as such. Expectation was that not many would be affected, because you in general should be using either os.path or pathlib when working with paths and in such usage the change has no effect.

I didn't, and don't, consider this to be big enough change to warrant calling the release RF 7. We've never said we'd be following semver strictly. More importantly, DataDriver and other possible tools that are affected by this would need to be updated anyway.

Snooz82 commented 1 year ago

Sure i will. And i did implement it a bad way. i am ok with keeping it.

Snooz82 commented 1 year ago

fixed in 1.8.0