Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robot 6.1 compatibility #88

Closed
pianofab opened this issue Jun 12, 2023 · 6 comments
Closed

Robot 6.1 compatibility #88

pianofab opened this issue Jun 12, 2023 · 6 comments
Labels
bug Something isn't working
Milestone

Comments

@pianofab
Copy link

pianofab commented Jun 12, 2023

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
Copy link
Author

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

robotframework/robotframework#4786

@pekkaklarck
Copy link

Most likely caused by this change:
robotframework/robotframework#4596

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

@Snooz82
Copy link
Owner

Snooz82 commented Jun 13, 2023

@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
Copy link

pekkaklarck commented Jun 13, 2023

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

  • Path is superior to str when working with paths.
  • We changed parsing to use Paths and it would be kind of stupid and inconsistent if TestSuite would convert source to str later.

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
Copy link
Owner

Snooz82 commented Jun 13, 2023

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

@Snooz82
Copy link
Owner

Snooz82 commented Jun 13, 2023

fixed in 1.8.0

@Snooz82 Snooz82 closed this as completed Jun 13, 2023
@Snooz82 Snooz82 added this to the 1.8.0 milestone Jul 27, 2023
@Snooz82 Snooz82 added the bug Something isn't working label Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants