Skip to content
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

Stevie Spiegl RPS #2115

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
4359d10
first commit
S-Spiegl May 7, 2022
1a24ee3
added app.rb and testing_infrastructure_spec
S-Spiegl May 7, 2022
cea74c9
commit check
S-Spiegl May 7, 2022
43edfb8
added play.erb
S-Spiegl May 7, 2022
e071a99
added Player model, '/play' route, and redirected '/name'
S-Spiegl May 7, 2022
392b36c
added play_game_spec and tests
S-Spiegl May 7, 2022
b136975
the model and controller are on speaking terms
S-Spiegl May 7, 2022
3be0405
game works. needs more tests
S-Spiegl May 7, 2022
0736433
some refactoring, several tests still need writing
S-Spiegl May 7, 2022
8bfb302
replaced buttons with images and updated tests accordingly
S-Spiegl May 7, 2022
23d4805
added capybara tests for various outcomes
S-Spiegl May 8, 2022
c1a4456
added background image for results screen
S-Spiegl May 8, 2022
0a48d3e
getting new errors
S-Spiegl May 8, 2022
af14b6c
come back here for comments/questions
S-Spiegl May 8, 2022
7fc16b4
refactored controller, took out comments
S-Spiegl May 8, 2022
f0c7b17
committing before changing computers
Jun 5, 2022
6ced6bc
Update README.md
S-Spiegl Jul 20, 2022
598460d
Update README.md
S-Spiegl Jul 20, 2022
d7f237f
Update README.md
S-Spiegl Jul 20, 2022
b331630
moved styling to stylesheet
S-Spiegl Aug 4, 2022
0162508
Merge branch 'main' of https://github.com/S-Spiegl/rps-challenge
S-Spiegl Aug 4, 2022
737ec52
set images as row, moved more logic to css
S-Spiegl Aug 4, 2022
e8cacd9
tidied up css for all views
S-Spiegl Aug 4, 2022
82d354a
finished tidying up css
S-Spiegl Aug 4, 2022
06a966c
Update README.md
S-Spiegl Aug 4, 2022
633c77b
added movies
S-Spiegl Aug 4, 2022
ddb2463
Merge branch 'main' of https://github.com/S-Spiegl/rps-challenge
S-Spiegl Aug 4, 2022
5dcf235
set up databases
S-Spiegl Aug 4, 2022
c5a6c4f
database accepting booleans for each go to represent wins, draws and …
S-Spiegl Aug 4, 2022
bb949f7
incrementing won, drew, lost columns in postgres
S-Spiegl Aug 4, 2022
9d44f07
renamed game to round;
S-Spiegl Aug 4, 2022
88bf62a
keeps track of score and prints to screen
S-Spiegl Aug 5, 2022
1353f7a
about to branch off for deployment
S-Spiegl Aug 5, 2022
dae923e
still undeployed
S-Spiegl Aug 5, 2022
6f8e164
Update README.md
S-Spiegl Aug 6, 2022
177486c
switched to improving tests branch
S-Spiegl Aug 6, 2022
1c2215f
mocking for round_spec passing
S-Spiegl Aug 6, 2022
06e98e2
tidied up round_spec
S-Spiegl Aug 6, 2022
f3b1de1
tests working
S-Spiegl Aug 6, 2022
1cb42bb
Merge pull request #1 from S-Spiegl/improving-tests
S-Spiegl Aug 6, 2022
d143894
merged
S-Spiegl Aug 6, 2022
da77118
set up test database
S-Spiegl Aug 6, 2022
db0dba3
added procfile
S-Spiegl Aug 7, 2022
2eab430
updated gemfile
S-Spiegl Aug 7, 2022
63b1b62
updated gemfile again
S-Spiegl Aug 7, 2022
2d23f71
testing if PG is the issue
S-Spiegl Aug 7, 2022
508234a
testing if PG is the issue
S-Spiegl Aug 7, 2022
a74ace6
fixing PG
S-Spiegl Aug 7, 2022
66baab1
fixing PG still
S-Spiegl Aug 7, 2022
64d1602
fixing PG still
S-Spiegl Aug 7, 2022
a34292f
fixing PG still
S-Spiegl Aug 7, 2022
10ba08e
fixing PG still
S-Spiegl Aug 7, 2022
b7773b9
fixing PG still
S-Spiegl Aug 7, 2022
1a95f63
fixing PG still
S-Spiegl Aug 7, 2022
72c4d4f
fixing PG still
S-Spiegl Aug 7, 2022
63bb33e
fixing PG still
S-Spiegl Aug 7, 2022
cebb680
still trying to establish connection
S-Spiegl Aug 7, 2022
852c43f
still trying to establish connection
S-Spiegl Aug 8, 2022
c252626
got PG working by transferring some of the logic into the round.rb, b…
S-Spiegl Aug 8, 2022
0401afd
successfully deployed. Now need to edit table functionality
S-Spiegl Aug 8, 2022
e0db0da
Update README.md
S-Spiegl Aug 8, 2022
2d98a80
Update README.md
S-Spiegl Aug 8, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 4 additions & 20 deletions app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,17 @@ class RockPaperScissors < Sinatra::Base
erb :form
end

post '/name' do
p params
post '/play' do
session[:player] = params[:player]
$player = Player.new(params[:player])
redirect '/play'
end

post '/name_play_again' do
p params
session[:player] = params[:player]
$player = Player.new(params[:player])
redirect '/play_again'
end

get '/play' do
@player_name = $player.name
#tried to get the welcome message into name... but no luck
erb :play
end

get '/play_again' do
post '/play_again' do
session[:player] = params[:player]
$player = Player.new(params[:player])
@player_name = $player.name
#tried to get the welcome message into name... but no luck
erb :play_again
end

Expand All @@ -49,10 +37,6 @@ class RockPaperScissors < Sinatra::Base
erb :battle
end

# run the model in the controller. Take args such as computer (contained in model)
# player choice (a param), and use them with the if/else statement in the engine
# to return a winner which can be interpolated in the view

# Start the server if this file is executed directly (don't change the line below)
run! if app_file == $0
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great file, concise, good use of class names and instance and global variables

5 changes: 0 additions & 5 deletions lib/game.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,4 @@ def engine
"You did not choose wisely."
end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to see the creation of a WEAPONS constant in this file. Could the engine method be refactored to reduce repetition - perhaps the use of a hash?


#replace the strings with a method, either winning_message or losing_message
#each will run sample on an array of three messages
#or use the phrases to suggest another go

end
4 changes: 2 additions & 2 deletions spec/features/enter_names_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
end

scenario 'player is taken to a new page to start the game' do
sign_in_and_play
sign_in
expect(current_path).to eq('/play')
end

scenario 'player can see their name on the play page' do
sign_in_and_play
sign_in
expect(current_path).to eq('/play')
expect(page).to have_content('Alien')
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear, concise, good indentation

Expand Down
35 changes: 6 additions & 29 deletions spec/features/play_game_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,66 +5,43 @@

feature 'allows player to play a game of RockPaperScissors' do
scenario 'the player can choose from three buttons' do
sign_in_and_play
sign_in
expect(page).to have_xpath('/html/body/form[1]/input[1]')
expect(page).to have_xpath('/html/body/form[2]/input[1]')
expect(page).to have_xpath('/html/body/div/form/input[1]')
end
#the above is a workaround because I can't seem to check for a link
#in the same way I was able to check for a button before I turned them
#into images... not sure how good it is though because you need to know
#the xpath before writing the tests...

scenario 'a player can return to the play screen to have another go' do
sign_in_and_play
sign_in
click_on(id='rock')
expect(page).to have_button('Have another go?')
end


scenario 'clicking a button takes the player to the winner page' do
sign_in_and_play
sign_in
click_on(id='rock')
expect(current_path).to eq '/battle'
end

scenario 'rock beats scissors' do
allow_any_instance_of(Computer).to receive(:weapon).and_return(:scissors)
sign_in_and_play
sign_in
click_on(id='rock')
expect(page).to have_content "You chose wisely."
end

scenario 'paper beats rock' do
allow_any_instance_of(Computer).to receive(:weapon).and_return(:paper)
sign_in_and_play
sign_in
click_on(id='rock')
expect(page).to have_content "You did not choose wisely."
end

scenario 'scissors beats paper' do
allow_any_instance_of(Computer).to receive(:weapon).and_return(:paper)
sign_in_and_play
sign_in
click_on(id='scissors')
expect(page).to have_content "You chose wisely."
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really clear test scenarios, really enjoyed reading these if I'm honest. Good takeaway for my tests in the future, thank you!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, good testing, well done for using stubs effectively here


#why no need for srand here in the stubs above?
#what's the difference between stubbing here and the doubling
#in the tests for computer_spec???

#would these tests be better off in the model???



#functionality to add:



#I'd like to route to a different image depending on the result
#but that seems like too much for now...
#credit unsplash photographers



end
2 changes: 1 addition & 1 deletion spec/features/web_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
def sign_in_and_play
def sign_in
visit('/')
fill_in 'player', with: 'Alien'
click_on('Submit name!')
Expand Down
12 changes: 0 additions & 12 deletions spec/game_spec.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,4 @@
require 'game'

describe Game do
xit 'paper beats scissors' do
end
xit 'scissors beats rock' do
end
xit 'rock beats scissors' do
end
xit 'two of the same draw' do
end
xit 'displays a congratulatory message if the player wins' do
end
xit 'taunts the player if they lose' do
end
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty file? I think you left the spec for this somewhere else? Then maybe you could delete this file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be good to consider what you might have tested in here if you had the time - you could even comment some ideas and come back to it at a later stage when you want to make improvements, especially the logic in Game is an important one.

3 changes: 0 additions & 3 deletions spec/player_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,4 @@
it "should return player's name" do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better indentation here. Clear class names and tests. Nice one man!

expect(player.name).to eq('Clodius')
end

xit 'allows a player to choose a weapon' do
end
end
2 changes: 1 addition & 1 deletion views/battle.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ text-align: center;}
<h1>You chose <%= @player_weapon %>...<br></h1>
<h1>Computer chose <%= @computer_weapon %>...<br><h1>
<h1><%= @message %><h1>
<form action='/name_play_again' method='post'>
<form action='/play_again' method='post'>
<button type="submit" style="padding: 5px 10px">Have another go?</button>
</form>
</body>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same again. Takes maybe a day or two to write the code, but people would have to read it an unspecified amount of time

2 changes: 1 addition & 1 deletion views/form.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ h1 {text-align: center; font-family: arial; padding-top: 20px}
</style>

<h1>Enter Name</h1>
<form action='/name' method='post'>
<form action='/play' method='post'>
<input type="text" name="player" style="padding: 5px 15px"><br><br>
<button type="submit" style="padding: 5px 10px">Submit name!</button>
</form>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation.. naughty naughty!

1 change: 0 additions & 1 deletion views/play.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ border='2' name='player_choice' type='submit' id='paper'></>
<input type="hidden" name="player_choice" value="paper" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to indent everything! Makes it easier to read for others ;) Maybe even have blank lines between the parts that don't relate i.e. After each form to show where one stops and the other starts (this is only when you know a code review will be done)

</form>
<div style="margin: 0px auto; width: 400; height: 267; overflow: hidden; border: 2px solid black">
<!-- margin: 0px auto saved me here, nothing else would center the div -->
<form action='/battle' method='post'>
<input type='image' src='https://source.unsplash.com/c18q3myyHLU' width='400'
name='player_choice' type='submit' id='scissors'></>
Expand Down