-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create Test_edge_creation #119
Conversation
|
||
var self = this; | ||
|
||
describe('Issue: Found class name null', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace "Issue:" for "Bug #116:" so it will be listed among the other bugs in the build log, example: https://travis-ci.org/appscot/sails-orientdb/jobs/69971485#L1258
Hey @tommykennedy, made few inline comments. Because the file doesn't end with |
I have updated the file with your instructions and appended .js to the filename. |
So it seems your test is actually passing (https://travis-ci.org/appscot/sails-orientdb/jobs/70070905#L1265):
Can it be that your model definition is slightly different from the one used in the test? |
I ran the test on my machine and it fails it with the error. Im using the following configuration below. OrientDB Server v2.0.1 "name": "sails-orientdb", |
Can you try upgrading your OrientDB server to v2.0.12 to dismiss any potential OrientDB change? The only thing that changed in master branch after v0.10.55 was updating dependencies, you can try that locally... but it's unlikely it would cause this issue. |
I upgraded to 2.0.12 and deleted sails-orientdb and reinstalled it npm install sails-orientdb. The exact output I get is.
Error (E_UNKNOWN) :: Encountered an unexpected error |
I ran the test script on another machine with Orient 2.0.12 and npm install sails-orientdb. I created the test script in /test/integration-orientdb/bugs and executed npm test. Same error Did you use the last updated 116-test_edge_creation.js in the test? |
I figured out how to turn debugging on and the output is below. Im not sure why answer gets an @Class property within the embedded type. Also why is there another type field with value document. I would have expected this to be in the @type field. Bug #116: Found class name null |
Dario Have you tried running the test script on your local machine? I build passed on travis for me also. Im totally stumped as it has failed on 2 separate machines now. |
I was just doing that :), the test passed against OrientDB v2.0.10:
Which OS are you using btw? My machine is a Mac while travis runs on linux (not sure which). |
Im running Mac OS X Yosemite 10.10.3 My debug output looks strange. Let me download 2.0.10 and check that. |
I'm still on Mavericks but I would be surprised if that was the source of our different results. Meanwhile I've submitted tommykennedy#1 in an attempt to fix the issue. Can you please try it and report back? |
…ass` if it's null or empty to allow user's submitted classes
Good to hear that! When sails-orientdb retrieves records from OrientDB it gets that metadata. It attempts to clean it where possible though I don't think it's cleaning it for embedded records... It seems the data is being added but it's really just saving what was retrieved. I've made an improvement to the workaround (10299b1), please test. Once you merge tommykennedy#1 I'll be able to merge this PR. |
appscot#116: attempt to fix error on edge creation
Thanks @tommykennedy for the report and test! |
No problem happy to contribute and I learned a lot to contribute more in the future Sent from my iPhone
|
Hi Dario
I hope this test is attached now
Tom
EDIT: adds failing test for #116