Split testing into its own project #235

schveiguy · 2021-05-25T01:58:22Z

This PR splits out all the integration-based testing into its own test project. Only a few tests were true unittests and did not require a full database connection to perform the test (which is a natural consequence of creating a database connection library).

The rest have been added to a subproject called integration-tests. You can run these via dub run :integration-tests

At this point, we are using vibe exclusively for the tests. We still need to perform the integration tests with Phobos sockets, that will be a future enhancement.

Also, the troublesome app.d has been moved to its own project called testconn (it needs testing as well).

schveiguy · 2021-05-25T01:58:51Z

ping @SingingBush

relevant.

schveiguy · 2021-05-25T02:05:48Z

source/mysql/connection.d

@@ -97,28 +93,6 @@ Prepared prepareFunction(Connection conn, string name, int numArgs)
 	return prepare(conn, sql);
 }

-///


Note, this unittest was not actually documented. And even if it were, it would have been terrible documentation (using scopedCn for the connection which is completely internal).

schveiguy · 2021-05-25T02:34:21Z

TODO: have to investigate spacing. My editor I used for a lot of this didn't obey the .editorconfig. grrrr.....

SingingBush

Are checks for version(DoCoreTests) are redundant now? There was a bunch of version checks that may not be needed with the way the code is now structured.

.github/workflows/integration-testing.yml

integration-tests/source/mysql/test/common.d

SingingBush · 2021-05-25T08:41:06Z

There are two files in my branch that I believe should be copied into this pull request (with tweaks):

integration-tests/docker-compose.yml
integration-tests/README.md

It should make it trivial for people to check the project out and run the tests with no prior setup (except to have docker) and a readme file specific to the test project means that the readme in the root of the project can be used to just put information that's relevant to people that want to use mysql-native as a dependency.

schveiguy · 2021-05-25T12:58:55Z

I might need your help on docker, I'm not well-versed in it.

schveiguy · 2021-05-25T17:39:00Z

Are checks for version(DoCoreTests) are redundant now?

Yeah, I think so. Working through the version logic, if you define MYSQLN_CORE_TESTS as opposed to MYSQLN_TESTS, the only test that is run is the basic connection test. Which is pretty useless.

I'm thinking we should get rid of these version statements, and rely on unit-threaded to allow running of individual tests. But maybe as a future PR, don't want to distract too much from the sheer copy-paste-ness of this PR.

schveiguy · 2021-05-25T21:11:15Z

So I've run into a weird problem. With dub, if you have a dependency with an optional dependency, you can't optionally depend on that optional dependency.

I added a second config to omit the dependency on vibe-core, and it still pulls in vibe-core.

I think I will need to create a third project that tests vibe, and make the vibe-core dependency optional. Ugh...

SingingBush · 2021-05-25T23:00:23Z

So I've run into a weird problem. With dub, if you have a dependency with an optional dependency, you can't optionally depend on that optional dependency.

I added a second config to omit the dependency on vibe-core, and it still pulls in vibe-core.

I think I will need to create a third project that tests vibe, and make the vibe-core dependency optional. Ugh...

isn't it something you can sort with configurations? like in this: https://github.com/SingingBush/mysql-native/blob/pipeline/integration_testing/tests/dub.sdl

dependency "mysql-native" path="../"

configuration "default" {
    subConfiguration "mysql-native" "library"
}

configuration "use-vibe" {
    subConfiguration "mysql-native" "library"
    dependency "vibe-core" version="~>1.16.0" optional=false
}

schveiguy · 2021-05-26T00:32:00Z

No, it doesn't work. I'm going to file a dub bug, but I have a crappy workaround at the moment. You'll see.

schveiguy · 2021-05-26T00:47:03Z

@SingingBush why aren't we running windows/macos integration tests? Are those not possible?

based on configuration.

schveiguy · 2021-05-26T01:32:42Z

Added the dub issue: dlang/dub#2136

schveiguy · 2021-05-26T01:36:42Z

@SingingBush I've noticed that the "Install dependencies on Ubuntu" never gets run for integration tests. Any idea why? It seems like it's not needed anyway.

SingingBush · 2021-05-26T08:51:47Z

@SingingBush I've noticed that the "Install dependencies on Ubuntu" never gets run for integration tests. Any idea why? It seems like it's not needed anyway.

good point that may not be needed anywhere. I think it used to be needed for vibe. Can probably come out

reflect the same actions as current tests

schveiguy · 2021-05-26T12:41:15Z

why aren't we running windows/macos integration tests? Are those not possible?

testing confirms, windows and macos do not support docker, and therefore cannot run an instance of mysql or mariadb. This is a shame, but I'm OK with it for now.

time building my branch in my repo, and the PR.

running tests (and update broken links)

.github/workflows/dub.yml

reflect how the tests can be run.

schveiguy · 2021-05-26T14:45:30Z

@SingingBush can you take a look at the docker file and integration test readme that I just added? I have not tested the docker image, I don't really know how to use docker. Note that I used the user/password in the default testConnectionStr.txt file for convenience (this is most likely what they will want to use).

SingingBush · 2021-05-26T14:51:59Z

that looks fine, cheers. After this is all merged I'll use it anyway so any issues then I'll push a commit.

schveiguy · 2021-05-26T16:04:40Z

Pretty satisfied with this. The only thing I need to figure out is how to restructure the dub file so that it doesn't spit out tons of warnings.

SingingBush · 2021-05-26T17:05:48Z

Pretty satisfied with this. The only thing I need to figure out is how to restructure the dub file so that it doesn't spit out tons of warnings.

Use silenceWarnings in build requirements. in dub.json this would be done with:

    "buildRequirements": [
		"silenceWarnings"
    ]

schveiguy · 2021-05-27T14:03:09Z

OK, I found another solution using "sourceLibrary", now the ugly warnings only appear in integration-tests, which I'm not so worried about.

I think this is ready to merge.

BTW, @SingingBush feel free to open a PR to add your name to the author/copyright

(mostly for people who want to run actions on their local branches)

schveiguy · 2021-05-27T14:32:36Z

@SingingBush can you explain the "dub upgrade" portions of the workflow? We don't need to dub upgrade anything, because there is no checked-in dub.selections.json file. So I'm confused why this is there.

I'm also not clear what the caching thing is, but I can probably ask webfreak about that on discord.

SingingBush · 2021-05-28T15:26:54Z

@SingingBush can you explain the "dub upgrade" portions of the workflow? We don't need to dub upgrade anything, because there is no checked-in dub.selections.json file. So I'm confused why this is there.

I'm also not clear what the caching thing is, but I can probably ask webfreak about that on discord.

Feel free to remove the dub upgrade bit. Sooner this is merged the better. Can then perhaps get both #187 and #220 merged in and do another release before you rebase/merge #214 and bump the major release version.

schveiguy · 2021-05-28T15:30:46Z

I checked with webfreak, and the dub upgrade is somewhat misnamed, as it caches the entire .dub folder for subsequent runs, in addition to running dub upgrade, as if you were storing it locally on your PC. A good test would be to run it with and without the dub upgrade to see how much time it saves.

I'll leave it there for now, it's not hurting anything (~~though I think it never runs the second dub upgrade~~ Edit: it does run on windows).

schveiguy added 2 commits May 24, 2021 15:45

Try to split out tests and test dependencies into separate projects

c0314bf

Integration tests now runnable

619332e

schveiguy added 2 commits May 24, 2021 22:00

Remove extraneous main function and extra imports that are no longer

592e218

relevant.

these shouldn't have been added.

51aa526

schveiguy commented May 25, 2021

View reviewed changes

This was referenced May 25, 2021

#233 integration tests are not unit tests #234

Closed

Replace mainSourceFile with excludedSourceFile in unittest build #229

Closed

SingingBush reviewed May 25, 2021

View reviewed changes

.github/workflows/integration-testing.yml Outdated Show resolved Hide resolved

integration-tests/source/mysql/test/common.d Show resolved Hide resolved

schveiguy force-pushed the splittests branch from e20a3ea to 61967ac Compare May 26, 2021 00:53

Add phobos tests. Work around dub bug that doesn't allow dependencies

d498e25

based on configuration.

schveiguy force-pushed the splittests branch from 61967ac to d498e25 Compare May 26, 2021 00:58

schveiguy force-pushed the splittests branch 2 times, most recently from a1c2fc7 to 2e4cb37 Compare May 26, 2021 12:24

Remove ubuntu "extra packages", update commented-out mysql-8 tests to

4723082

reflect the same actions as current tests

schveiguy force-pushed the splittests branch from 2e4cb37 to 4723082 Compare May 26, 2021 12:39

schveiguy added 2 commits May 26, 2021 09:06

Try only building on PRs and pushes to master. Otherwise, github wastes

466fc39

time building my branch in my repo, and the PR.

Remove obsolete test runner. Update README to reflect new ways of

a148f58

running tests (and update broken links)

SingingBush reviewed May 26, 2021

View reviewed changes

.github/workflows/dub.yml Show resolved Hide resolved

schveiguy added 2 commits May 26, 2021 10:21

run library tests in integration test applications. Update README to

48b0193

reflect how the tests can be run.

Add docker config and readme on how to use it.

2411644

schveiguy force-pushed the splittests branch 4 times, most recently from 8543e5d to 71055c2 Compare May 26, 2021 15:52

Fix spacing. Run test utility in integration tests.

6efb6e3

schveiguy force-pushed the splittests branch from 71055c2 to 6efb6e3 Compare May 26, 2021 15:54

Use target type of sourceLibrary to help run unittest configuration.

6e7ccda

schveiguy mentioned this pull request May 27, 2021

sourceLibrary package's unittest config should be used when testing dependee dlang/dub#1735

Open

schveiguy marked this pull request as ready for review May 27, 2021 14:03

Add workflow_dispatch to workflows to allow manual triggering of builds

ef97eb1

(mostly for people who want to run actions on their local branches)

schveiguy merged commit 323ee58 into mysql-d:master May 28, 2021

schveiguy deleted the splittests branch May 28, 2021 16:04

SingingBush mentioned this pull request May 28, 2021

move test code that connects to a db into separate module #233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split testing into its own project #235

Split testing into its own project #235

schveiguy commented May 25, 2021

schveiguy commented May 25, 2021

schveiguy May 25, 2021

schveiguy commented May 25, 2021

SingingBush left a comment

SingingBush commented May 25, 2021

schveiguy commented May 25, 2021

schveiguy commented May 25, 2021

schveiguy commented May 25, 2021

SingingBush commented May 25, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021

SingingBush commented May 26, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021 •

edited

Loading

SingingBush commented May 26, 2021

schveiguy commented May 26, 2021

SingingBush commented May 26, 2021

schveiguy commented May 27, 2021

schveiguy commented May 27, 2021

SingingBush commented May 28, 2021

schveiguy commented May 28, 2021 •

edited

Loading

Split testing into its own project #235

Split testing into its own project #235

Conversation

schveiguy commented May 25, 2021

schveiguy commented May 25, 2021

schveiguy May 25, 2021

Choose a reason for hiding this comment

schveiguy commented May 25, 2021

SingingBush left a comment

Choose a reason for hiding this comment

SingingBush commented May 25, 2021

schveiguy commented May 25, 2021

schveiguy commented May 25, 2021

schveiguy commented May 25, 2021

SingingBush commented May 25, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021

SingingBush commented May 26, 2021

schveiguy commented May 26, 2021

schveiguy commented May 26, 2021 • edited Loading

SingingBush commented May 26, 2021

schveiguy commented May 26, 2021

SingingBush commented May 26, 2021

schveiguy commented May 27, 2021

schveiguy commented May 27, 2021

SingingBush commented May 28, 2021

schveiguy commented May 28, 2021 • edited Loading

schveiguy commented May 26, 2021 •

edited

Loading

schveiguy commented May 28, 2021 •

edited

Loading