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

Assert more about the test server fixture #140

Merged
merged 4 commits into from
Jan 28, 2020
Merged
Changes from 2 commits
Commits
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
82 changes: 71 additions & 11 deletions python/test_perforce.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,78 @@ def test_fixture(capsys):
repo = P4Repo()
assert repo.info()['serverAddress'] == port

# There should be a sample file checked into the fixture server
# Returns [metadata, contents]
content = repo.perforce.run_print("//depot/file.txt")[1]
assert content == "Hello World\n"

shelved_change = repo.perforce.run_describe('-sS', '3')
assert len(shelved_change) > 0, "Shelved changelist was missing"
assert shelved_change[0]['depotFile'] == ['//depot/file.txt'], "Unexpected files in shelved changelist"
# To change the fixture server, uncomment the next line and put a breakpoint on it.
# Log in using details printed to stdout (port/user)
# Make changes to the p4 server then check in the new server.zip
# To change the fixture server, uncomment the line below with 'store_server' and put a breakpoint on it
# Run unit tests in the debugger and hit the breakpoint
# Log in using details printed to stdout (port/user) via p4v or the command line
# Make changes to the p4 server
# Continue exection so that the 'store_server' line executes
ca-johnson marked this conversation as resolved.
Show resolved Hide resolved
ca-johnson marked this conversation as resolved.
Show resolved Hide resolved
# Replace server.zip with new_server.zip
# Update validation code below to document the new server contents

# store_server(repo, 'new_server.zip')

# Validate contents of server fixture @HEAD
depotfiles = [info['depotFile'] for info in repo.perforce.run_files('//...')]
depotfile_to_content = {depotfile: repo.perforce.run_print(depotfile)[1] for depotfile in depotfiles}
assert depotfile_to_content == {
"//depot/file.txt": "Hello World\n",
"//stream-depot/main/file.txt": "Hello Stream World\n"
Comment on lines +106 to +107
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Content of files

}

# Check submitted changes
submitted_changes = [change for change in repo.perforce.run_changes('-s', 'submitted')]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a long-form flag instead of -s, for readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately not, it means status

submitted_changeinfo = {change["change"]: repo.perforce.run_describe(change["change"])[0] for change in submitted_changes}
# Filter info to only contain relevant keys for submitted changes
submitted_changeinfo = {
change: {key: info.get(key)
for key in ['depotFile', 'desc', 'action']}
for change, info in submitted_changeinfo.items()
}
assert submitted_changeinfo == {
'1' :{
'action': ['add'],
'depotFile': ['//depot/file.txt'],
'desc': 'Initial Commit'
},
'2' :{
'action': ['add'],
'depotFile': ['//stream-depot/main/file.txt'],
'desc': 'Initial Commit to Stream\n'
},
'6' :{
'action': ['edit'],
'depotFile': ['//depot/file.txt'],
'desc': 'modify //depot/file.txt\n'
},
}
Comment on lines +119 to +135
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If tests fail, it shows you a rich diff of exactly which bits were different


# Check shelved changes
shelved_changes = [change for change in repo.perforce.run_changes('-s', 'pending')]
shelved_changeinfo = {change["change"]: repo.perforce.run_describe('-S', change["change"])[0] for change in shelved_changes}
Copy link
Contributor

Choose a reason for hiding this comment

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

Long form for -S?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately not, it means get info for shelved files on this change
hopefully this is implied from the var it is being assigned to

# Filter info to only contain relevant keys for submitted changes
shelved_changeinfo = {
change: {key: info.get(key)
for key in ['depotFile', 'desc', 'action']}
for change, info in shelved_changeinfo.items()
}
assert shelved_changeinfo == {
'3' :{
'action': ['edit'],
'depotFile': ['//depot/file.txt'],
'desc': 'Modify file in shelved change\n',
# Change content from 'Hello World\n' to 'Goodbye World\n'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully the comment suffices here
If there are more complex shelved changes created we could look at fetching the content of the shelved change and including it here. Omitted for the sake of simplicity for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually on second though this might be really useful to include the content, as it means we can put this static info in a global var for the unit tests and refer to it from throughout, avoiding potential mistakes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe in a future PR...

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be up for making that, it's been a while since I python'd.

},
'4' :{
'action': ['delete'],
'depotFile': ['//depot/file.txt'],
'desc': 'Delete file in shelved change\n',
},
'5' :{
'action': ['add'],
'depotFile': ['//depot/newfile.txt'],
'desc': 'Add file in shelved change\n',
},
}

def test_head():
"""Test resolve of HEAD changelist"""
Expand Down