Getting started as a new dev/contributor - notes & issues

[draknor]

I'll use this discussion thread as a place to document some of my thoughts / ideas / reactions as a new developer+contributor to hostedgpt. I'll post new comments for each item to get things separated!

[draknor]

Setting up my local development environment - couple of improvements:

• Specify username & password via environment variables (with defaults) in config/database.yml for dev + test databases.
• Will also want to allow environment variable for db host for production
• Document in README.md the default db username, password, and test databases (that way as an initial developer, I can just follow the steps in README without having to hunt through the code)

[krschacht]

Ah, yes, all rails apps support the DATABASE_URL env, both for development and production. The production configs (render, fly, heroku) already set this. But this should definitely be in the README! The format for this variable should be postgres://username:password@localhost/hostedgpt_development

[krschacht]

But it's common to not require a username or password for development databases so we should also document that you can customize just the database name with: HOSTEDGPT_DEV_DB

[krschacht]

#454

[draknor]

UI - Settings -- Language Models and API Services are pretty much unreadable in Dark Mode:

[krschacht]

Dang, good catch. That's a new feature and it was an oversight. I already have an Issue to re-design this whole thing but I added a note about dark mode: #432

If you want to tackle the dark mode piece, you're welcome to, but I'll do the larger re-design of it.

[krschacht]

Actually, let me go ahead and fix this right now. It'll just take a few minutes. I don't want to leave this broken in main.

[krschacht]

Done, I'll merge this in once CI finishes
#455

[draknor]

Branch strategy & philosophy -- I pulled down the repo yesterday, and ran tests on branch main (commit ab2a5ac7) and got 11 failed tests.

While it's still < 1.0 -- is the approach to do "little stuff" directly in main, and use feature branches for the "bigger stuff"? (and so main won't always be a 100% green?

[krschacht]

Main should definitely always be green, in fact CI runs every time anything is merged into main and if CI fails I get an email and fix it right away. Can you share more about the failures? I suspect it's something related to your dev environment, but I'm definitely a big surprised because, off the top of my head, I can't think what would even fail for you in dev.

We should basically always create branches. Normally I only merge directly into main if I'm fixing a broken PR, updating the README, or happen to be out and doing something on my phone/ipad and need to do it quickly.

[draknor]

All the failures were related to UI stuff -- test export attached.
test_results_hostedgpt.zip

Here are the errors:

test_clicking_the_assistant_name_in_the_nav_column_starts_a_new_conversation
Minitest::UnexpectedError: Capybara::ElementNotFound: Unable to find visible xpath "./*" within #
test/application_system_test_case.rb:67:in assert_shows_tooltip' test/system/messages/nav_column_test.rb:159:in block in 'test/application_system_test_case.rb:67:in assert_shows_tooltip' test/system/messages/nav_column_test.rb:159:in block in '

test_clicking_edit_activates_a_form_with_cancel,_clicking_cancel_reverts
Minitest::UnexpectedError: Selenium::WebDriver::Error::ElementNotInteractableError: element not interactable
(Session info: chrome-headless-shell=126.0.6478.126)
test/system/conversations/messages/edit_test.rb:37:in block in 'test/system/conversations/messages/edit_test.rb:37:in block in '

test_clicking_edit,_changing_text,_saving_updates_the_message_text,_previous_navigates_back
Minitest::UnexpectedError: Selenium::WebDriver::Error::ElementNotInteractableError: element not interactable
(Session info: chrome-headless-shell=126.0.6478.126)
test/system/conversations/messages/edit_test.rb:47:in block in 'test/system/conversations/messages/edit_test.rb:47:in block in '

test_edit_icon_shows_a_tooltip
Minitest::UnexpectedError: Capybara::ElementNotFound: Unable to find visible xpath "./*" within #
test/application_system_test_case.rb:67:in assert_shows_tooltip' test/system/conversations/messages/edit_test.rb:32:in block in 'test/application_system_test_case.rb:67:in assert_shows_tooltip' test/system/conversations/messages/edit_test.rb:32:in block in '

test_there_is_no_edit_icon_beneath_messages_with_images
Minitest::Assertion: expected to find visible css "#message_240941427 [data-role='message-edit']" but there were no matches. Also found "", which matched the selector but not all filters. test/system/conversations/messages/edit_test.rb:27:in `block in '

[krschacht]

@draknor It looks like all of these are failing capybara tests. That is probably because of a timing issue. I spent a lot of time trying to remove all the "sleep X" from the system tests in order to minimize timing issues, but there are still lots of unavoidable moments in the tests where it will wait up to default_wait_time seconds until an element appears and if it doesn't see it by then, it fails.

The easiest way to determine you have a timing issue is to pick any of the failing tests and just run it in isolation — confirm that it succeeds. e.g. bin/rails test test/system/conversations/messages/edit_test.rb:27

Basically, the system tests are running too slowly on your machine. My machine is only an M2 with 4 cores so what I do is run them separately: I first run bin/rails test to run all non-system tests and then I run bin/rails test:system to run the system tests. This still parallelizes them but that seems to lesson the CPU load a bit.

Alternatively, you can lesson it even more by doing:

PARALLEL_WORKERS=1 bin/rails test:system

They take a lot longer to run so you can try incrementally increasing that until you see when things start failing. This is one of the many reasons why DHH recently declared that system tests are a failure :)

https://world.hey.com/dhh/system-tests-have-failed-d90af718

They sure are a pain to maintain and keep going. I'm not ready to give up on them yet but I'm trying to utilize then as little as possible.

[draknor]

Ok, that makes sense - I've had some similar issues in past projects but I didn't do heavy front-end so it wasn't as big of an issue. If I come up with any clever solutions I'll let you know, but for now I'll just plan to ignore the capybara failings.

[draknor]

I'm new to collaborative dev on Github (one of the handicaps of having traditionally been a solo dev!) -- what's your workflow for me to make code changes locally and submit them for review & merging into the main repo? Something like this -- Pull Request Workflow with Git ?

[krschacht]

Yep, that's exactly right.