X Tutup
Skip to content

Fix tests on firefox#1808

Merged
emanlove merged 17 commits intorobotframework:masterfrom
Brownies:fix-tests-on-firefox
Jan 18, 2023
Merged

Fix tests on firefox#1808
emanlove merged 17 commits intorobotframework:masterfrom
Brownies:fix-tests-on-firefox

Conversation

@Brownies
Copy link
Member

@Brownies Brownies commented Dec 22, 2022

  • Drop Robot Framework 3 and PythonLibCore 2 support, new version of one combined with older version of the other causes type conversion problems.
  • Add Firefox to CI runs
  • Adjust tests and tags so that they run on Chrome and Firefox

Copy link
Member

@emanlove emanlove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some inline comments ...

if: matrix.python-version != '3.8' && matrix.python-version != 'pypy-3.7'

- name: Run tests with normal Chrome and with Python 3.7
if: matrix.python-version == '3.7'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be running headless Chrome and not "normal" Chrome.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Thinking out loud ..] Wondering about the matrix of python versions and browsers versions. Not sure the origin of these specific combinations but it may be worth checking overall what we are testing to see if there are gaps. For example here it look like we have switched from testing normal Chrome with Python 3.8 to Python 3.7.

And with the expanding versions of Python to 3.9, 3.10 how does these fit into the mix. @aaltat always had the strong opinion of limiting the overall testing to just the latest versions. He has very good reason for this - essentially limited resources and time- although I keep wanting expand the testing matrix. I'll say I have no strong reasons for this nor have I overcome the real issue of limited resources. One idea I had was to make this a separate GitHub workflow file which would not "block" - this stil doesn't resolve the real issue of support. We both agree that being able to publicly state what is supported - i.e. what we have tested against - is important and should be done.
This background info I think is important just so all understand all the previous thinking and work that has gone into the current test setup. This way we can build upon that experience and not struggle through the same without changes or improving upon that experience. I'm not opposed to change but am wanting to learn from others experiences in order to make better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I will fix this to normal chrome. 3.8 was not tested previously, despite what some of the task names state. This is because 3.8 was not(and is not) included in the matrix.

I would prefer to get rid of this if python-version == xx && yy=zz and just use the matrix to define browsers and python+rf versions. Maybe even Selenium versions in the future, though this is not realistic at the moment. I realize this results in a lot of jobs very quickly but as far as I can tell Github Actions are free for public repositories and the only limit is 20 concurrent jobs. I think this would further encourage making current tests more robust or not-flaky by fixing timing issues and such, and also to help create robust tests in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditions on python version I believe were to limit the overall matrix. I'm good with expanding it. I would say possibly in another pull request.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah definitely not doing that in this pull request.

@emanlove emanlove added this to the v6.1.0 milestone Dec 30, 2022
@emanlove emanlove merged commit 9e0e2ef into robotframework:master Jan 18, 2023
@emanlove emanlove added enhancement priority: medium acknowledge To be acknowledged in release notes labels Apr 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge To be acknowledged in release notes enhancement priority: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup