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.

Sign up for GitHub

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

Jump to bottom

test: remove index column from test assertion #374

Merged
merged 2 commits into from
Jan 21, 2021
Merged

test: remove index column from test assertion #374

merged 2 commits into from
Jan 21, 2021

Conversation

jenhagg
Copy link
Collaborator

@jenhagg jenhagg commented Jan 21, 2021 •
edited
Loading

Purpose

Fix failing integration test - id column is now an index, so isn't in the .columns attribute. Code changes were introduced in #365

What the code is doing

Updated the expected list of column headers

Testing

Ran pytest -m "not db" locally

Time estimate

2 min

@jenhagg jenhagg requested a review from dmuldrew January 21, 2021 19:57
@jenhagg jenhagg self-assigned this Jan 21, 2021
@jenhagg jenhagg added this to the Serenity Now milestone Jan 21, 2021
@rouille
Copy link
Collaborator

rouille commented Jan 21, 2021

Should we test that execute_table.index.name and scenario_table.index.name equal id?

rouille
rouille approved these changes Jan 21, 2021
View reviewed changes
Copy link
Collaborator

@rouille rouille left a comment

Choose a reason for hiding this comment

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

Good catch

@jenhagg
Copy link
Collaborator Author

jenhagg commented Jan 21, 2021

Found via @dmuldrew's integration test branch

@jenhagg jenhagg merged commit c595d1d into develop Jan 21, 2021
@jenhagg jenhagg deleted the jon/csv_test branch January 21, 2021 20:12
@ahurli ahurli mentioned this pull request Mar 11, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rouille rouille rouille approved these changes

@dmuldrew dmuldrew Awaiting requested review from dmuldrew

Assignees

@jenhagg jenhagg

Labels
None yet
Projects
None yet
Milestone
Serenity Now
Development

Successfully merging this pull request may close these issues.
2 participants
@jenhagg @rouille