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

Prepare for react-lite → React switch #9938

Merged
merged 6 commits into from
Sep 3, 2018

Conversation

martinpitt
Copy link
Member

This picks a bunch of commits from PR #9263 which are safe and fine for react-lite as well, and removes some conflicts to current master. Let's land this separately so that it's easier to eventually get in the complete switch.

@martinpitt
Copy link
Member Author

These are all @mareklibra 's changes, and I approve them. I can't formally do it here, as it's nominally my PR. I manually tested terminal (including resizing), selectors, the ReactDOM.render affected pages etc., and it all works just fine.

@mareklibra mareklibra mentioned this pull request Aug 29, 2018
44 tasks
@@ -83,6 +83,19 @@ class TestOVirtMachines(MachineCase):
self.hack_for_missing_vdsm()
self.pass_install_dialog()

# mimics pressing keys
def _setVal(self, selector, value):
Copy link
Member

Choose a reason for hiding this comment

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

We should use Browser.set_input_text for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, and I added a separate commit to robustify set_input_text() like Marek did in the _setVal helper function.

@@ -239,6 +239,7 @@ def set_input_text(self, selector, val):
self.set_val(selector, "")
self.focus(selector)
self.key_press(val)
b.wait_val(selector, val)
Copy link
Member

Choose a reason for hiding this comment

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

self.wait_val

Copy link
Member Author

Choose a reason for hiding this comment

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

Argh, fixed.

martinpitt and others added 6 commits August 31, 2018 14:45
Wait until the element that received the typed string really has the
intended value. This avoids race conditions with slower keyboard event
handlers.
Needed for upgrade from react-lite to React 16.4

- React is more strict regarding component nesting.
- Checkboxes can't be set via testlib's `set_checked()` but
  via i.e. clicking (`input.checked` attribute is not working)
- Tests mimic key presses to set value of `<input>` instead of
  directly setting property
Moved from `react` to `react-dom` package.
Moved from `react` to `react-dom` package.

Closes cockpit-project#9938
@martinpitt martinpitt dismissed mvollmer’s stale review August 31, 2018 15:30

should be good now. PTAL?

@martinpitt
Copy link
Member Author

Two storage test failures on ubuntu-1604: https://fedorapeople.org/groups/cockpit/logs/pull-9938-20180831-235353-9c47b50d-verify-ubuntu-1604/log.html#201

Retrying for comparison.

@martinpitt
Copy link
Member Author

Actually @mvollmer already approved up there, just wanted the wait_val typo fixed (which broke tests). ubuntu-1604 udisks bugs are unrelated, he's working on them. So this can land now IMHO.

@martinpitt martinpitt merged commit a0b6ac3 into cockpit-project:master Sep 3, 2018
@martinpitt martinpitt deleted the react branch September 3, 2018 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants