hack4impact-uiuc / h4i-recruitment

H4I recruitment platform
https://h4i-recruitment.vercel.app
17 stars 3 forks source link

Continuous integration #370

Closed coconut750750 closed 4 years ago

coconut750750 commented 4 years ago

Resolves #368

vercel[bot] commented 4 years ago

This pull request is being automatically deployed with Vercel (learn more). To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hack4impact/h4i-recruitment/fnfxtxsh8 ✅ Preview: https://h4i-recruitment-git-bw-continuous-integration.hack4impact1.vercel.app

josh-byster commented 4 years ago

Awesome, thanks @coconut750750! Any idea why the styling is off in the deployed link?

coconut750750 commented 4 years ago

Hm, I don't know why the styling is off. It seems like it's failing to load the style.css in the static/ directory. Any ideas why or how to fix this?

josh-byster commented 4 years ago

Yeah, you're absolutely right. After some investigation, it looks like since we switched to using the root project directory instead of running next in the context of the src directory, the static folder wasn't getting recognized. One of the exceptions to using src with this change is that the static or public folder must be outside the src/ folder, so I made that edit.

API tests are all failing since we moved to using /api/... for our routes. I think it would be great if we can figure out a way to do this without having to change every single test written. There's gotta be a way in supertest to specify a base API URI for what we're doing, but some preliminary searching turned up nothing. Would you mind digging into this a bit @coconut750750? Worst case we just change everything from /user to /api/user for example, but it would be great if we could keep this abstracted away.

coconut750750 commented 4 years ago

@josh-byster I dont think there is a way. On SO, there are some solutions for remote APIs: https://stackoverflow.com/questions/13754105/reuse-supertest-tests-on-a-remote-url.

I guess we should just change all of them to hit /api/

josh-byster commented 4 years ago

While unfortunate, that makes sense. Thanks for looking into it and making the fixes, @coconut750750!