Last commit broke connect

[michaelfliegner]

haskey(conn_data, "database") && push!(dns, string("dbname=", conn_data["database"]))
was deleted, but is needed, as LibPQ expects an option dbname instead of database

same with username / user

[essenciary]

@michaelfliegner Thanks for this! Sorry, my bad, I copied from the MySQL adaptor and missed these differences!

[essenciary]

Mmm, actually the solution is not 100% correct - the objective was to allow defining connection parameters using ENV variables. Now these are not available for dbname and user.

[michaelfliegner]

I see. can fix that to support You, if you want.
what about Franku's CI job?

[essenciary]

@michaelfliegner I pushed the changes on master - can you please check that it works fine? If good, I'll tag a new version.

No idea re the CI job... we need to check.

[essenciary]

There are no test jobs configured. Looks like they haven't been setup for Postgres.

[michaelfliegner]

I can provide a CI-Workflow soon, a test stub and just a test on connect :-) Others can easily contribute tests then. OK?

[FrankUrbach]

There are no test jobs configured. Looks like they haven't been setup for Postgres.

There is an PR open but now useless because of the ongoing work made in the past months. The main parts are ready and need only to be copied into a new PR based on the actual version. This is really straight forward. The first test are also there.
We have to think about the question which tests should evaluated central (written in SearchLight.jl) which each adapter has to fullfill and which tests are specific to the adapter. Therefore it would be a good idea to implement a modul for the testcases so each adapter can easily using this. Just my thoughts. I'm courious to hear your thoughts.

[essenciary]

@FrankUrbach can you please detail how you see this module?

[FrankUrbach]

@essenciary This modul should contain the all general functions which every adapter should implement in the same fashion. E.g. connect, disconnect, query etc. So it would be possible to get the same results from each and every adapter. If there are differences we can pin it at the documentation. So we have the ability to test the conformity of the api you've created in the past.

[michaelfliegner]

couldn't it be submodule of SearchLight?

[FrankUrbach]

Yes of course. That was the idea behind my answer.

[essenciary]

That's what I try to understand. Why would a testing module become part of the library? Why not a file in test/ or a different package? How would the submodule work and what is the benefit of doing it this way? In general the test/ folder and runtest.jl use a different Project.toml file so as to not mix test dependencies with package ones.

[FrankUrbach]

From the point of view of the adapter Searchlight is a different package an is not reachable if you don't know where the source is placed. In order to get what we want, we must know in each and every environment where the source of the tests of SearchLight are located. Is this possible? I don't think so. Therefore the idea to create a test module inside Searchlight. I can imagine that you are not happy about a module which isn't necessary in production. But to be honest, this isnt that much code we will need for testing.
I hope it picture is now more clear.

[essenciary]

The way I see it, there are multiple things to consider - and we need clarity re what we want to achieve.

1/ if we talk about pure SearchLight.jl tests then we're talking about Unit tests. Because SearchLight itself does not connect to DB, we test the various functions and make sure they return what's expected.

2/
a) when we talk about testing the adapters, we can also do Unit tests.
b) but here we also talk about integration tests where we actually connect to a database and run various queries and test the results.

For 1/ unit testing can be put directly into SearchLight tests.
For 2/ a) the unit tests will be specific to the individual adapter, so again, they would go into the adapter's tests
For 2/ b) we can assume that integration tests can be reused across all adapters so we want to reuse the tests. Here I'm pretty sure we can register a package that includes the integration tests logic, ex SearchLightTests.jl and use it.

[FrankUrbach]

Now we are at the same page. If you set up the package SearchlightTest.jl I will try to fill it with live. The rest is work.

[essenciary]

@FrankUrbach @michaelfliegner I have created a new blank repo at https://github.com/GenieFramework/SearchLightTests.jl and invited you as maintainers.

[michaelfliegner]

@FrankUrbach <https://github.com/FrankUrbach> I take it, that You take the lead, so I wait for Your suggestions. Michael

…

On Tue, Aug 16, 2022 at 9:34 AM Adrian Salceanu ***@***.***> wrote: @FrankUrbach <https://github.com/FrankUrbach> @michaelfliegner <https://github.com/michaelfliegner> I have created a new blank repo at https://github.com/GenieFramework/SearchLightTests.jl and invited you as maintainers. — Reply to this email directly, view it on GitHub <#14 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC6Y2USJBRVDUWP5V6TDSLLVZNAADANCNFSM56E47A6A> . You are receiving this because you were mentioned.Message ID: ***@***.***>