Get All Tests Working

[codesplode]

As I've been learning squash, I bumped into a couple issues running the tests. 1) Two of the tests do not provide a name for the test entity 2) The embedded MySQL was failing on windows by timing out because it could not open the windows event log. Upon reading the past issues on wix-embedded-mysql I determined this concern is removed in later versions, so I updated the dependency

I have been able to fix both these issues and can now run all tests on all databases successfully. I will submit two pull requests (one for each item above).

[orangy]

1. There is a default name for the entity inferred from the type name:

    override val compoundName = Identifier(name ?: javaClass.simpleName.removeSuffix("Table"))

So if you don't provide a name it should make it from class name. What failure did you see? On our CI it passes: https://teamcity.jetbrains.com/project.html?projectId=KotlinTools_Squash&tab=projectOverview

[orangy]

2. Merged

[codesplode]

Hi @orangy,

That is great to know and I appreciate you mentioning that.

The tests that were failing are: DefinitionTests.tableExists https://github.com/orangy/squash/blob/83c9b4f057d7ba9380688e9315f9a39244798b96/squash-core/test/org/jetbrains/squash/tests/DefinitionTests.kt#L20

DefinitionTests.unnamedTableWithQuotesSQL https://github.com/orangy/squash/blob/83c9b4f057d7ba9380688e9315f9a39244798b96/squash-core/test/org/jetbrains/squash/tests/DefinitionTests.kt#L31

Per your comment, I converted the test table for tableExists to a class and that also completed successfully. For the tests to work, I have to either convert these test table definition to a class as I just did or add a name. I don't see how they would work with a fresh checkout of the code. The 2nd test also works as a class, but as you can imagine with that table name, it looks a bit messy:

    @Test fun tableExists() {
        class TestTable : TableDefinition() {
            val id = integer("id").primaryKey()
            val name = varchar("name", length = 42)
        }

        val testTable = TestTable()

        withTables(testTable) {
            assertEquals (true, exists(testTable))
        }
    }

    @Test fun unnamedTableWithQuotesSQL() {
        class `UnnamedTableWithQuotesSQL$TestTable$1` : TableDefinition() {
            val id = integer("id").primaryKey()
            val name = varchar("name", length = 42)
        }

        val testTable = `UnnamedTableWithQuotesSQL$TestTable$1`()

        withTransaction {
            connection.dialect.definition.tableSQL(testTable).assertSQL {
                "CREATE TABLE IF NOT EXISTS ${quote}UnnamedTableWithQuotesSQL\$TestTable$1${quote} (id INT NOT NULL, name VARCHAR(42) NOT NULL, CONSTRAINT ${quote}PK_unnamedTableWithQuotesSQL\$TestTable$1${quote} PRIMARY KEY (id))"
            }
        }
    }

[codesplode]

Haha, thanks @orangy,

Let me know if you would prefer the class version and I'll switch that up and send a pull request.

[orangy]

Can you provide information on what exactly failed in tests? Since I can't reproduce it, I don't quite understand the problem.

[codesplode]

Sure, with your added information above, here is the issue:

The runtime exception that was occurring is this one: https://github.com/orangy/squash/blob/83c9b4f057d7ba9380688e9315f9a39244798b96/squash-core/src/org/jetbrains/squash/definition/Name.kt#L11

The code, as it was before edits for me, was the following (Note that TestTable is a val with object definition instead of a class):

    @Test fun tableExists() {
        // FROM MIKE - No class definition, so the name is not defaulted, causing an error
        val TestTable = object : TableDefinition() {
            val id = integer("id").primaryKey()
            val name = varchar("name", length = 42)
        }

        withTables(TestTable) {
            assertEquals (true, exists(TestTable))
        }
    }

    @Test fun unnamedTableWithQuotesSQL() {
        // FROM MIKE - No class definition, so the name is not defaulted, causing an error
        val TestTable = object : TableDefinition() {
            val id = integer("id").primaryKey()
            val name = varchar("name", length = 42)
        }

        withTransaction {
            connection.dialect.definition.tableSQL(TestTable).assertSQL {
                "CREATE TABLE IF NOT EXISTS ${quote}unnamedTableWithQuotesSQL\$TestTable$1${quote} (id INT NOT NULL, name VARCHAR(42) NOT NULL, CONSTRAINT ${quote}PK_unnamedTableWithQuotesSQL\$TestTable$1${quote} PRIMARY KEY (id))"
            }
        }
    }

Because TestTable is not a class, the there was no name to get defaulted and the runtime exception mentioned above was occurring.

Hope this helps!

[orangy]

What is JVM you're running on? Before fixing an issue we might need to know the cause…

[orangy]

I set a breakpoint in the Identifier class and get this: tableExists$TestTable$1, so it gives a name in my setup.

[codesplode]

Sure, I'm on JDK 10.0.2 from https://jdk.java.net/10/

I just saw your breakpoint comment and I see what you mean! I had no idea that would even quite function that way haha. That helps me understand the confusion better 🥇

[orangy]

Can you check it with JVM8? I suspect it's the difference.

[codesplode]

Sure, I'll have to redownload it so give me a bit and I will confirm.

Thanks for the assist!

[codesplode]

You are absolutely right. JDK 8 functions without any exceptions. What direction should we take this from here haha?

I'm happy to do any modifications based on the direction you choose, but it definitely looks like a change in JDK behavior here. As a side note, I've also confirmed that JDK 11 has the same issue.

[orangy]

I need to investigate the difference. But not now, since it's about midnight here already :)

[codesplode]

Understood. I appreciate the support given the time and I will try to look it up as well.

[orangy]

It looks like a bug fixed in newer JRE. Doc on simpleName says "Returns an empty string if the underlying class is anonymous.", but it didn't in this case for JRE8, and it does for JRE11. So we need to fix autodetection of compound name for anonymous classes. Fix is coming.

[orangy]

Fix pushed, thanks!

[codesplode]

Haha, I had just started doing the same thing after realizing the jdk documentation confirmed the change. You beat me to it sir. Nice moves 👍

[codesplode]

Last night, I got a chance to run all the tests on JDK 8, 10, and 11 and confirmed they all work nicely.