Skip to content

Commit

Permalink
[data] Adding in better json checking in test logging (ray-project#48036
Browse files Browse the repository at this point in the history
)

## Why are these changes needed?
Previously this test simply checked if the first and last characters of
a line were `{` or `}` to assert whether or not they were JSON lines.
This updates the test to instead attempt to parse them as JSONs which is
much more robust.

## Related issue number

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for
https://docs.ray.io/en/master/.
- [ ] I've added any new APIs to the API Reference. For example, if I
added a
method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a
few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

---------

Signed-off-by: Matthew Owen <mowen@anyscale.com>
Signed-off-by: Matthew Owen <omatthew98@berkeley.edu>
Co-authored-by: Scott Lee <scottjlee@users.noreply.github.com>
  • Loading branch information
2 people authored and Jay-ju committed Nov 5, 2024
1 parent c726981 commit 70a01e3
Showing 1 changed file with 11 additions and 10 deletions.
21 changes: 11 additions & 10 deletions python/ray/data/tests/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ def test_custom_config(reset_logging, monkeypatch, tmp_path):
def test_json_logging_configuration(
capsys, reset_logging, monkeypatch, shutdown_only, propagate_logs
):
import json

monkeypatch.setenv("RAY_DATA_LOG_ENCODING", "JSON")
ray.init()

Expand All @@ -147,20 +149,19 @@ def test_json_logging_configuration(
log_contents = file.read()

# Validate the log is in JSON format (a basic check for JSON)
assert all(
log_line.startswith("{") and log_line.endswith("}")
for log_line in log_contents.splitlines()
)
messages = []
for log_line in log_contents.splitlines():
log_dict = json.loads(log_line) # will error if not a json line
messages.append(log_dict["message"])

assert '"message": "ham"' in log_contents
assert '"message": "turkey"' in log_contents
assert "ham" in messages
assert "turkey" in messages

# Validate console logs are in text mode
console_log_output = capsys.readouterr().err
assert not any(
log_line.startswith("{") and log_line.endswith("}")
for log_line in console_log_output.splitlines()
)
for log_line in console_log_output.splitlines():
with pytest.raises(json.JSONDecodeError):
json.loads(log_line)

assert "ham" in console_log_output
assert "turkey" not in console_log_output
Expand Down

0 comments on commit 70a01e3

Please sign in to comment.