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

Fix lots of unresolved imports / undefined stuff #4385

Closed
wants to merge 11 commits into from

Conversation

benjaoming
Copy link
Contributor

It worries me to see all the issues in 0.14.x :/ Luckily most issues are in tests.

    And a tooltip should appear on the username box only # kalite/distributed/features/steps/login.py:77
    And a tooltip should appear on the username box only # kalite/distributed/features/steps/login.py:77 10.280s
      Traceback (most recent call last):
        File "/home/ubuntu/virtualenvs/venv-2.7.9/lib/python2.7/site-packages/behave/model.py", line 1173, in run
          match.run(runner.context)
        File "/home/ubuntu/virtualenvs/venv-2.7.9/lib/python2.7/site-packages/behave/model.py", line 1589, in run
          self.func(context, *args, **kwargs)
        File "/home/ubuntu/ka-lite/kalite/distributed/features/steps/login.py", line 79, in impl
          assert check_single_popover(context, "username")
        File "/home/ubuntu/ka-lite/kalite/distributed/features/steps/login.py", line 116, in check_single_popover
          popover = find_id_with_wait(context, "id_{item}-popover".format(item=item))
        File "/home/ubuntu/ka-lite/kalite/testing/behave_helpers.py", line 165, in find_id_with_wait
          return _find_elem_with_wait(context, (By.ID, id_str), **kwargs)
        File "/home/ubuntu/ka-lite/kalite/testing/behave_helpers.py", line 207, in _find_elem_with_wait
          EC.presence_of_element_located(by)
        File "/home/ubuntu/virtualenvs/venv-2.7.9/lib/python2.7/site-packages/selenium/webdriver/support/wait.py", line 75, in until
          raise TimeoutException(message, screen, stacktrace)
      TimeoutException: Message:
      Stacktrace:
          at FirefoxDriver.prototype.findElementInternal_ (file:///tmp/tmp7gSEFb/extensions/[email protected]/components/driver-component.js:10271)
          at FirefoxDriver.prototype.findElement (file:///tmp/tmp7gSEFb/extensions/[email protected]/components/driver-component.js:10280)
          at DelayedCommand.prototype.executeInternal_/h (file:///tmp/tmp7gSEFb/extensions/[email protected]/components/command-processor.js:12274)
          at DelayedCommand.prototype.executeInternal_ (file:///tmp/tmp7gSEFb/extensions/[email protected]/components/command-processor.js:12279)
          at DelayedCommand.prototype.execute/< (file:///tmp/tmp7gSEFb/extensions/[email protected]/components/command-processor.js:12221)
…ture, it seems something messes it up -- this is a fix to just eliminate this part of the test and move on
@benjaoming
Copy link
Contributor Author

Notice! All the BDD tests are running now, except what I've disabled.

@benjaoming
Copy link
Contributor Author

Yay, we're now at this stage: Node 1 (non-BDD) times out.

@benjaoming
Copy link
Contributor Author

Good news: All the tests are now running on my local machine (non BDD) ! The BDD tests are already running on Circle.

@MCGallaspy
Copy link
Contributor

It worries me to see all the issues in 0.14.x

Why? Hadn't we decided to address those issues in (then) develop as a practical compromise, given that as you say the issues appear to be only test related? Trying to address them here is duplicating that effort from other branches in an incompatible way.

@benjaoming
Copy link
Contributor Author

The sentence was incomplete, it worries me that we have all these issues in the tests! It worries me that we released 0.14 without being able to run tests, locally nor in Travis/Circle.

The fixes I applied were very obvious stuff... broken imports etc...

@MCGallaspy
Copy link
Contributor

Yeah I get that it is worrisome, but you're rehashing the discussion we already had about this very topic, wherein we decided that it was better to release something that we had tested extensively by hand than to delay even further by working on protracted test issues.

And my point about incompatibility is not that your changes are non-obvious, but that you're taking a different approach than we already took in 0.15.x and develop to get the tests working. Like this. Arguably if you're going to have an empty step you might as well delete it, otherwise it gives the wrong impression that something is being tested and passing when it's not.

My opinion is that we should focus on the more pressing 0.15.x and develop branches than worry about 0.14.x any more. Thus my question -- why worry about failing tests on 0.14.x? What value does it add? Or if you must worry about 0.14.x tests, why didn't you say anything back when we were discussing this before the 0.14.0 release? This is an honest question -- do you feel you were not included in that discussion?

@benjaoming
Copy link
Contributor Author

I asked a couple of times if anyone was gonna run the tests locally before releasing. I did say that, and I didn't follow up on it until now when I was doing a change for 0.14 and I started wondering about the test situation.

Then I found that the tests had lots of errors, and I'm suggesting that we don't do any patch releases without having functioning tests. I assumed that the test fixes from 0.15 would be too incompatible to backport.

Rather than complaining, I made a PR, I hope you can see the difference here ;)

@benjaoming benjaoming closed this Sep 9, 2015
@benjaoming benjaoming removed the has PR label Sep 9, 2015
@benjaoming benjaoming deleted the fix-tests branch July 27, 2016 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants