Thanks for your feedback, Dave! We can put the tests under the regression directory. I think that makes sense. I'm not picturing these tests being module specific, but we may want to enable running it as a separate suite of tests.
Thanks for the callout about the port and title. We'll make sure those are pulled from config or that the pgAdmin server is spun up by the test with specific values. I have a couple ideas about why the test might not have been running for you. I think the patch we attached didn't spin up its own pgAdmin yet and it definitely doesn't fill in username/password if your app is running that way. That's part of the WIP-ness :-P -Tira Hi On Thu, Jan 12, 2017 at 10:41 PM, George Gelashvili <ggelashvili(at)pivotal(dot)io> wrote: > here's the patch we forgot to attach. Also, you can see work on our branch > at: > https://github.com/pivotalsoftware/pgadmin4/tree/pivotal/acceptance-tests > > On Thu, Jan 12, 2017 at 5:26 PM, George Gelashvili > <ggelashvili(at)pivotal(dot)io> > wrote: >> >> Hi there, >> >> We are working on browser-automation-based acceptance tests that exercise >> pgAdmin4 the way a user might. Nice! >> The first "connect to database" test works, but at the moment depends on >> Chrome and chromedriver. We would appreciate feedback on any possible >> license or code style issues at this point, as well as any thoughts on >> adding this sort of test to the codebase. A few thoughts: - If these tests are to run as part of the regression suite, the framework for them should live under that directory. - Are any of the tests likely to be module-specific? If so, they should really be part of the relevant module as the regression tests are. If they're more general/less tightly coupled, then I don't see a problem with them residing where they are. - Please take care not to include changes to .gitgnore files that aren't relevant to the rest of us. - The port number is hard-coded in the test. - You've hard-coded the string "pgAdmin 4". We've tried to keep that title as a config option in config.py, so you should pull the string from there rather than hard-coding it. - The connect test fails for me (Mac, Python 2.7). I have a suspicion that this may be because when the test starts chromedriver, OS X prompts the user about whether a listening port should be opened, but the tests don't wait (though, I tested with 3 servers configured and it failed with the same error on the second and third as well, long after I clicked OK on the prompt): Traceback (most recent call last): File "/Users/dpage/git/pgadmin4/web/acceptance/test_connects_to_database.py", line 32, in runTest self.assertEqual("pgAdmin 4", self.driver.title) AssertionError: 'pgAdmin 4' != u'localhost' - Please keep tests in the pgadmin. namespace (pgadmin.acceptance.??). - It looks like running a single test won't work yet (because of TestsGeneratorRegistry.load_generators('pgadmin.%s.tests' % arguments['pkg'])) Thanks! -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
