-
Notifications
You must be signed in to change notification settings - Fork 235
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
15/07/22 #236
base: main
Are you sure you want to change the base?
15/07/22 #236
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
--require spec_helper | ||
--format documentation |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,36 @@ | ||
require 'sinatra/base' | ||
require './lib/chitter' | ||
|
||
class Chitter < Sinatra::Base | ||
get '/test' do | ||
'Test page' | ||
end | ||
|
||
get '/' do | ||
erb :index | ||
end | ||
|
||
get '/peeps' do | ||
@peeps = Peep.all | ||
# @time = Peep.time | ||
erb :peeps | ||
end | ||
|
||
post '/peeps' do | ||
Peep.create(peep: params[:peep], timestamp: params[:timestamp]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Passing the time in here is one way to do it but it's vulnerable to bugs. For example:
For such reasons, it's best to generate timestamps in your Model when you're about to add the peep to the database so that you're sure it will be accurate. I see that you were thinking along those lines in your commented out code in the To control the time in tests, you can stub the return value of |
||
|
||
# post '/bookmarks' do | ||
# Bookmark.create(url: params[:url], title: params[:title]) | ||
# redirect '/bookmarks' | ||
# end | ||
# @time = Peep.time | ||
@peeps = Peep.all | ||
erb :peeps | ||
end | ||
|
||
get '/peeps/add' do | ||
erb :"/peeps/add" | ||
end | ||
|
||
run! if app_file == $0 | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
-- To both chitter and chitter_test databases: | ||
CREATE TABLE peeps(id SERIAL PRIMARY KEY, message VARCHAR(60)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
-- To both chitter and chitter_test databases: | ||
ALTER TABLE peeps ADD COLUMN timestamp VARCHAR(60); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of the timestamp column type! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
require 'pg' | ||
|
||
class Peep | ||
def self.all | ||
if ENV['ENVIRONMENT'] == 'test' | ||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
|
||
result = connection.exec("SELECT * FROM peeps") | ||
result.map { |peep| peep['message'] } | ||
end | ||
|
||
def self.create(peep:, timestamp:) | ||
if ENV['ENVIRONMENT'] == 'test' | ||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
Comment on lines
+16
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're using this same piece of code in more than one place in this file. How might you get rid of this duplication? |
||
|
||
connection.exec("INSERT INTO peeps (message, timestamp) VALUES('#{peep}', #{timestamp})") | ||
Comment on lines
+2
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice and easy to read |
||
# connection.exec("INSERT INTO bookmarks (title, url) VALUES('#{title}', '#{url}') RETURNING id, url, title") | ||
end | ||
|
||
# def self.time | ||
# if ENV['ENVIRONMENT'] == 'test' | ||
# connection = PG.connect(dbname: 'chitter_test') | ||
# else | ||
# connection = PG.connect(dbname: 'chitter') | ||
# end | ||
|
||
# connection.exec("INSERT INTO peeps (timestamp) VALUES('#{Time.now}')") | ||
# end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
feature 'Viewing peeps' do | ||
scenario 'viewing related timestamp' do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like this test is just for viewing the peeps rather than timestamps, as the expected outcome is just a lil peep peep. I'm guessing you were halfway through adapting an existing test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice observation @Amy-O ! I agree, right now this test is not testing what it says it does. |
||
visit('/peeps/add') | ||
fill_in('peep', with: 'Peep peep!') | ||
click_button('Post') | ||
expect(page).to have_content "Peep peep!" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
feature 'Viewing homepage' do | ||
scenario 'visiting homepage' do | ||
visit('/') | ||
expect(page).to have_content "Welcome to Chitter!" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
feature 'Adding a peep' do | ||
scenario 'adding a peep' do | ||
visit('/peeps/add') | ||
fill_in('peep', with: 'Peep peep!') | ||
click_button('Post') | ||
expect(page).to have_content "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we be expecting the page to have "Peep peep!" as content? |
||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
require 'pg' | ||
require './lib/chitter' | ||
|
||
feature 'Viewing peeps' do | ||
scenario 'A user can see peeps' do | ||
# connection = PG.connect(dbname: 'chitter_test') | ||
|
||
# # Add the test data | ||
# connection.exec("INSERT INTO peeps VALUES(1, 'Peep one');") | ||
# connection.exec("INSERT INTO peeps VALUES(2, 'Peep two');") | ||
# connection.exec("INSERT INTO peeps VALUES(3, 'Peep three');") | ||
|
||
# Bookmark.create(website: "http://www.makersacademy.com") | ||
# Bookmark.create(website: "http://www.destroyallsoftware.com") | ||
# Bookmark.create(website: "http://www.google.com") | ||
Comment on lines
+6
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't think to add the bookmark stuff in comments for easier comparison! smart |
||
|
||
Peep.create(peep: 'Peep one') | ||
Peep.create(peep: 'Peep two') | ||
Peep.create(peep: 'Peep three') | ||
visit('/peeps') | ||
|
||
expect(page).to have_content "Peep one" | ||
expect(page).to have_content "Peep two" | ||
expect(page).to have_content "Peep three" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
require 'rspec' | ||
require 'simplecov' | ||
require 'simplecov-console' | ||
require 'timecop' | ||
|
||
require_relative './setup_test_database' | ||
|
||
|
@@ -41,6 +42,11 @@ | |
RSpec.configure do |config| | ||
config.before(:each) do | ||
setup_test_database | ||
|
||
# new_time = Time.local(2008, 9, 1, 12, 0, 0) | ||
# Timecop.freeze(new_time) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see you were trying to use Timecop to freeze the time during tests. Maybe it was tricky getting it to work? An alternative that doesn't require an additional gem is to use mocking, e.g.:
|
||
|
||
|
||
end | ||
|
||
config.after(:suite) do | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<h1> Welcome to Chitter! </h1> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<ul> | ||
<% @peeps.each do |peep| %> | ||
<li><%=peep%></li> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All the rest of your code is there to fulfil user story 3 (showing the timestamp) -- did you run out of time or did you get blocked? |
||
<% end %> | ||
</ul> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<form method="post" action="/peeps"> | ||
<input type="text" name="peep" placeholder="Add your thoughts" /> | ||
<input type="hidden"name="timestamp" value="<%=Time.now%>"> | ||
<input type="submit" value="Post" /> | ||
</form> |
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.
It's always a good idea to remove commented out code from code you submit for code review (anywhere, not just at Makers)