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

test: do not assume parent directory name #30

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

kapouer
Copy link
Contributor

@kapouer kapouer commented Jan 6, 2024

No description provided.

@@ -318,7 +319,7 @@ def test_logger_name_is_test(self):
def test_path_name_is_test(self):
logger.error('An error has occured')
json_record = json.loads(log_buffer.getvalue())
self.assertIn('json-log-formatter/tests.py', json_record['pathname'])
self.assertIn(os.path.basename(os.path.abspath('.')) + '/tests.py', json_record['pathname'])
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please tell when does this test break and how can it be reproduced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just by not having the name of the root directory of the project to be "json-log-formatter".

Copy link
Owner

Choose a reason for hiding this comment

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

There are no plans to rename the directory name, so I am not sure how it can help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For example, i made a debian package python3-json-log-formatter
The source of that package, when being built in debian infrastructure, is put in some directory like
python-json-log-formatter_0.5.2/
Since the tests are run from that directory, one of them fail.
The change I propose here let the test work independently of the name of the parent directory where the test is run.

Copy link
Owner

Choose a reason for hiding this comment

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

I see, that makes sense.

@marselester
Copy link
Owner

Could you please rebase your branch?

@marselester marselester merged commit bf41c57 into marselester:master Jan 6, 2024
6 checks passed
@marselester
Copy link
Owner

Thank you 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants