Add tests for findAllWorkspaceAndInstances #4120
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Apr 30, 2021
The test is currently failing with a missing database error. I think this is a problem with the test database setup rather than the specific test
mads-hartmann
force-pushed
the
mads/workspace-db-test
branch
from
edca096 to
d9262c3
Compare
Contributor
Author
|
/werft run 👍 started the job as gitpod-build-mads-workspace-db-test.3 |
Turns out that TypeORM column names are case sensitive when used for
ordering. The error message is terrible
Cannot read property 'databaseName' of undefined
In this case we used contextUrl and it should have been contextURL.
While column names in mysql are case insensitive I also fixed one other
occurrence of contextUrl even though it is not necessary; but using the
column names makes it easier to find when searching for references etc.
geropl
approved these changes
May 28, 2021
Member
geropl
left a comment
geropl
left a comment
There was a problem hiding this comment.
Nice find, thx for digging in and fixing it.
In general we could think about using Object.keys to be sure we use the exact same field name. But that's Out of scope for this PR I think.
MatthewFagan
pushed a commit
to trilogy-group/gitpod
that referenced
this pull request
Nov 22, 2021
* Add test for findAllWorkspaceAndInstances
The test is currently failing with a missing database error. I think
this is a problem with the test database setup rather than the specific
test
* Resolve failing test
Turns out that TypeORM column names are case sensitive when used for
ordering. The error message is terrible
Cannot read property 'databaseName' of undefined
In this case we used contextUrl and it should have been contextURL.
While column names in mysql are case insensitive I also fixed one other
occurrence of contextUrl even though it is not necessary; but using the
column names makes it easier to find when searching for references etc.
* Add a launch configuration for db-test
* Add more assertions to the test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on #3992 I was trying to add a few DB tests.
Before making changes to the code I wanted to create a basic test for the existing functionality, but unfortunately the test failed with a rather obscure error message
Turns out that TypeORM column names are case sensitive when used for ordering. In this case we used contextUrl and it should have been contextURL.
While column names in mysql are case insensitive I also fixed one other occurrence of contextUrl even though it is not necessary; but using the column names makes it easier to find when searching for references etc.
I also added a launch configuration for executing the db tests with the debugger enabled so you can step through them.