-
Notifications
You must be signed in to change notification settings - Fork 27
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
WIP: Update dependencies to work on recent node versions #300
base: develop
Are you sure you want to change the base?
Conversation
…(13) This fixes a build issue due to [email protected] which doesn't seem to build anymore on recent node version. Webdriver is depending on fiber, so updating the webdriver packages fixes this. Since newer versions of webdriver have removed browser.clik in favor of $(selector).click(), this also updates the integration tests for this change. Closes hotosm#295
Thanks so much for contributing @julienr ! As you can see there are some issues with the integration tests, which for me have been with the facebook and google logins. To address your points:
|
Looking a bit more into this
Now, finding the image associated with this upload (which is what's returned by the
Looking in the meta table, I think this is the corresponding metadata (although I'm unclear as to how one should find the metadata corresponding to an image. Here, the timestamps seem to match):
I feel what's in the meta table is what's the frontend is expecting the Looking at the history of hotosm/oam-api@84d3492#diff-647a467e336f16a2a502ee92eac324f2L286 I don't understand enough of the overall architecture to fully grasp the intent behind these changes, so it's very possible that I am missing something here. I could try assigning the whole |
Indeed, copying the metadata in the image in oam-api seems to help with the I'm not quite sure this is the right fix though, because I guess this metadata will not be present for images in the current prod db, so this would break backward compatibility. I feel getting these tests to pass again is going to require a number of changes. Do you know the last commits of oam-browser/oam-api for which these tests were passing ? Do you prefer that I try to fix everything at once in a big PR or we do it step-by-step in multiple small increments ? |
TODO
Background
I was trying to get started hacking on oam-browser and ran into some setup issues with some of the dependencies, mostly around webdriverio. It seems like the versions that oam-browser is using are not working anymore with recent node version (I'm testing on 13).
So I figured out I need to update some packages and the webdriverio ones seems to be the most problematic ones (they depend on an old
fibers
version that doesn't build anymore).Closes #295
Status
I'm almost there with the integration tests running locally against oam-api (I opened hotosm/oam-api#153 for a fix for oam-api).
The only tests that seem to fail are the one simulating a click on 'View image' after upload. But is this functionality supposed to work ? Because it seems the metadata returned by oam-api doesn't contain a uuid and oam-browser is looking for a uuid, in
StatusImage.js:imageClick
. I am testing against the develop branch of oam-api. Guidance on this would be appreciated.StatusImage.js issue
If I print the metadata that the
imageClick
function is receiving, I'm seeing this:This corresponds that what's stored in the
uploads
table of the DB, and the frontend is indeed getting that from the/uploads/{id}
route ofoam-api
.Now, what this function seems to expect is the actual image metadata as stored on S3 (in the
XXX_meta.json
file or in themetas
DB table.So I feel there is an issue there that the frontend is not calling the right oam-api route to get this metadata.
Question
develop
? Looking at the commits, seems the test are broken since a while. I'd be willing to help getting them green againStatusImage.js:imageClick
issue, I'd be happy