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

Fix the test for GMT_COMPATIBILITY=6 #454

Merged
merged 8 commits into from
May 25, 2020
Merged

Fix the test for GMT_COMPATIBILITY=6 #454

merged 8 commits into from
May 25, 2020

Conversation

seisman
Copy link
Member

@seisman seisman commented May 24, 2020 •
edited
Loading

Description of proposed changes

Clean up the global "gmt.conf" in the current directory, after ending the local session but before beginning the global session.

Fixes #453

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

@seisman seisman requested a review from weiji14 May 24, 2020 20:23
@seisman seisman added the maintenance Boring but important stuff for the core devs label May 25, 2020
@weiji14
Copy link
Member

weiji14 commented May 25, 2020

Wasnt't this what #432 (comment) was about... Oh well, I think you've documented it better here though.

Are you sure about removing the try...except block? It's more to ensure the file gets deleted when the test fails rather than when the test passes.

@vercel vercel bot temporarily deployed to Preview May 25, 2020 10:07 Inactive
@seisman
Copy link
Member Author

seisman commented May 25, 2020

Are you sure about removing the try...except block? It's more to ensure the file gets deleted when the test fails rather than when the test passes.

You're right. I assumed that the test always passes.

@vercel vercel bot temporarily deployed to Preview May 25, 2020 10:10 Inactive
seisman
seisman commented May 25, 2020
View reviewed changes
pygmt/tests/test_session_management.py
Comment on lines -46 to -48
with Session() as lib:
# revert gmt.conf back to GMT_COMPATIBILITY = 6
lib.call_module("set", "GMT_COMPATIBILITY 6")
Copy link
Member Author

Choose a reason for hiding this comment

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

These lines are not needed, as they only change the gmt.conf in the session directory, which will be deleted after the end() statement.

seisman
seisman commented May 25, 2020
View reviewed changes
pygmt/tests/test_session_management.py
Comment on lines +48 to +50
# Clean up the global "gmt.conf" in the current directory
assert os.path.exists("gmt.conf")
os.remove("gmt.conf")
Copy link
Member Author

Choose a reason for hiding this comment

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

Have to remove the "gmt.conf" in the current directory so that it won't affect other tests.

@seisman seisman merged commit 292c060 into master May 25, 2020
@seisman seisman deleted the gmtcompat6-test branch May 25, 2020 11:14
@seisman seisman added this to the 0.2.x milestone May 25, 2020
@weiji14 weiji14 modified the milestones: 0.2.x, 0.1.x Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weiji14 weiji14 weiji14 approved these changes

Assignees
No one assigned
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Milestone
0.1.x
Development

Successfully merging this pull request may close these issues.

Test test_gmt_compat_6_is_applied leaves a gmt.conf file in the current directory
2 participants
@seisman @weiji14