Re: Acceptance Tests against a browser (WIP)

Thanks, patch applied!

On Tue, Feb 21, 2017 at 10:12 PM, Atira Odhner <aodhner(at)pivotal(dot)io> wrote:
> Hi Dave,
>
> We fixed the flakiness issues that we saw (hopefully they are the same ones
> you were seeing.) by tearing down connections to the acceptance_test_db
> before attempting to drop it at the beginning of the test. Once we have
> access to the CI pipeline we can help out there to ensure the flakiness is
> gone.
>
> We wrapped the README at 80 characters, and removed the misleading function
> definition from the test.
>
> As far as the screenshots go, I'm more inclined to remove the screenshotting
> than to work on improving it. It currently only works when the failure is
> due to an AssertionError since that's what failureException relies on.
>
> We also renamed acceptance to feature_tests since 'acceptance' seemed
> ambiguous/redundant with 'regression'.
>
> Tira & Sara
>
>
> On Mon, Feb 13, 2017 at 9:36 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>> Hi,
>>
>> I've been playing with this for the last couple of hours, and I just
>> can't get it to work reliably;
>>
>> - A good percentage of the time the browser opens with a URL of
>> "data:," and does nothing more. This appears to happen if tests fail,
>> which still leaves server processes running in the background.
>>
>> - The connect_to_server test usually seems to work.
>>
>> - The template_selection_feature test usually does *not* work. I can't
>> see an obvious reason, but I suspect it's a race condition. What seems
>> to happen is that the function definition is entered, but not
>> registered by the UI, so the mSQL panel just ends up saying
>> "incomplete definition". Manually checking what was input proves that
>> everything is correct - and indeed, returning the SQL tab shows the
>> expected SQL.
>>
>> Other issues I noted:
>>
>> - The template_selection_feature test should just enter BEGIN/END.
>> What it currently enters is an entire function definition, when only
>> the body content is expected. E.g.
>>
>> self.page.fill_codemirror_area_with(
>> """BEGIN
>>
>> END;
>> """
>> )
>>
>> - Screenshots are being taken of failed tests:
>> 1) I've never actually seen any get saved
>> 2) They should be saved to the same directory as the test log, not /tmp
>> 3) They should have guaranteed unique names, and be mentioned in the
>> test output so the user can reference the image to the failure.
>>
>> The reason the last two items are important is that I've now got a
>> test server running the test suite with every supported version of
>> Python, for every supported database (well, almost, pending a couple
>> of fixes). I have separate workspaces for each Python version, and a
>> single test run might run every test 10 times, once for each database
>> server.
>>
>> - Please wrap the README at < 80 chars.
>>
>>
>>
>> On Thu, Feb 9, 2017 at 4:17 PM, Atira Odhner <aodhner(at)pivotal(dot)io> wrote:
>> > Hi Dave,
>> >
>> >> I think the problem was that the way you phrased it,
>> >
>> >
>> > You're right, we totally messed that up. We were talking about making 3
>> > patches and ended up making only 2 and forgot to reword that bit.
>> > Sorry about that.
>> >
>> > Here are the two patches for this change that resolves the
>> > AttributeError
>> > you were seeing. The first patch is identical to the patch of the same
>> > name
>> > in the other email thread.
>> >
>> >> We're used to
>> >> dealing with larger patchsets via the mailing list - typically as long
>> >> as you're clear about any dependencies, it shouldn't be a problem.
>> >
>> >
>> > Great! We'll try sending patchsets from now on and hopefully that
>> > resolves
>> > some of the issues we were seeing.
>> >
>> > Tira & George
>> >
>> > On Thu, Feb 9, 2017 at 9:28 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Thu, Feb 9, 2017 at 2:20 PM, Atira Odhner <aodhner(at)pivotal(dot)io>
>> >> wrote:
>> >> > Certainly. We did mention the dependency in the email. Would it be
>> >> > better
>> >> > to mention it in the patch name?
>> >>
>> >> I think the problem was that the way you phrased it, it sounded
>> >> optional ("an updated patch which does not include adding that test
>> >> helper in case you apply the show-tables patch first"). I think a
>> >> clear "This patch is dependent on patch Foo" would suffice.
>> >>
>> >> > Is there a better way for us to manage
>> >> > these changes? On other open source projects, I've seen github
>> >> > mirrors
>> >> > set
>> >> > up so that changes can be pulled in like branches rather then as
>> >> > patch
>> >> > applies. That would have avoided this situation since the parent
>> >> > commit
>> >> > would be pulled in with the same SHA from either pull request branch
>> >> > and
>> >> > git
>> >> > would not see it as a conflict.
>> >> >
>> >> > I'm rather new to dealing with patch files like this so I would love
>> >> > some
>> >> > tips.
>> >>
>> >> The Postgres project in general is quite conservative and stuck in
>> >> it's ways about how things are done (which is usually a good thing
>> >> considering you trust your data to the resulting code). We're used to
>> >> dealing with larger patchsets via the mailing list - typically as long
>> >> as you're clear about any dependencies, it shouldn't be a problem.
>> >> Some of us use tools like PyCharms for handling patches and helping
>> >> with reviews etc. which I guess replaces most, if not all of the
>> >> GitHub functionality over plain git.
>> >>
>> >> --
>> >> Dave Page
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >>
>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company