diff --git a/CHANGELOG.md b/CHANGELOG.md index 89a1bcc..78f37d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Keep output colors local to each `Outputter` so overlapping comparisons can use different color settings without changing colorama globals or report alignment ([#366](https://github.com/nasa/ncompare/issues/366)) ([**@sylvesterkaczmarek**](https://github.com/sylvesterkaczmarek)). - Run the unit test suite on pull requests targeting `develop` and `main` (previously only PRs based on `feature/**`/`issue/**` triggered tests). Integration tests, which require Earthdata credentials, are not run on pull requests and continue to run post-merge via `version-and-build.yml` ([#355](https://github.com/nasa/ncompare/issues/355)) ([**@danielfromearth**](https://github.com/danielfromearth)) - Internal cleanup with no change to comparison output: remove dead code, compute each variable's scale factor and attributes once instead of twice, and add missing Apache license headers ([#347](https://github.com/nasa/ncompare/issues/347)) ([**@danielfromearth**](https://github.com/danielfromearth)) - Compare ATL06 structure from committed structure-only fixtures instead of downloading granules, so the test needs no network or Earthdata credentials and its difference count no longer changes when ATL06 is reprocessed. Tests that reach live Earthdata services no longer run automatically, so an outage there cannot break `develop`; run them on demand with `pytest -m integration` ([#361](https://github.com/nasa/ncompare/issues/361)) ([**@danielfromearth**](https://github.com/danielfromearth)) diff --git a/ncompare/Comparison.py b/ncompare/Comparison.py index 0edc663..841d0f4 100644 --- a/ncompare/Comparison.py +++ b/ncompare/Comparison.py @@ -30,7 +30,6 @@ import h5py import netCDF4 import numpy as np -from colorama import Fore from ncompare.getters import ( get_and_check_variable_attributes, @@ -104,7 +103,7 @@ def run_through_comparisons(self) -> int: self._print_root_attributes() # Run through all the rest of the groups and variables, tallying differences along the way. - self.out.print(Fore.LIGHTBLUE_EX + "\nAll variables:", add_to_history=True) + self.out.print_header("\nAll variables:") self.out.side_by_side(" ", "File A", "File B", force_display_even_if_same=True) self._traverse_hierarchy() @@ -323,14 +322,14 @@ def _tally_attribute_difference( def _print_root_dimensions(self): # Show the dimensions of each file and evaluate differences. - self.out.print(Fore.LIGHTBLUE_EX + "\nRoot-level Dimensions:", add_to_history=True) + self.out.print_header("\nRoot-level Dimensions:") list_a = get_root_dims(self.file1) list_b = get_root_dims(self.file2) _, _, _ = self.out.lists_diff(list_a, list_b) def _print_root_groups(self): # Show the groups in each NetCDF file and evaluate differences. - self.out.print(Fore.LIGHTBLUE_EX + "\nRoot-level Groups:", add_to_history=True) + self.out.print_header("\nRoot-level Groups:") list_a = get_root_groups(self.file1) list_b = get_root_groups(self.file2) _, _, _ = self.out.lists_diff(list_a, list_b) @@ -346,7 +345,7 @@ def _print_root_attributes(self) -> None: ------- None """ - self.out.print(Fore.LIGHTBLUE_EX + "\nRoot-level Attributes:", add_to_history=True) + self.out.print_header("\nRoot-level Attributes:") attrs_a = get_root_attributes(self.file1) attrs_b = get_root_attributes(self.file2) @@ -369,14 +368,8 @@ def _print_summary(self): self.__print_summary_count_comparison_side_by_side("attribute", self.num_attribute_diffs) if self.num_attribute_diffs["difference_types"]: - self.out.print( - Fore.LIGHTBLUE_EX + "\nDifferences were found in these attributes:", - add_to_history=True, - ) - self.out.print( - Fore.LIGHTBLUE_EX + f"\n{sorted(self.num_attribute_diffs['difference_types'])}", - add_to_history=True, - ) + self.out.print_header("\nDifferences were found in these attributes:") + self.out.print_header(f"\n{sorted(self.num_attribute_diffs['difference_types'])}") def __print_summary_count_comparison_side_by_side( self, diff --git a/ncompare/printing.py b/ncompare/printing.py index 307d836..b1f00ee 100644 --- a/ncompare/printing.py +++ b/ncompare/printing.py @@ -28,6 +28,7 @@ import csv import re +import sys import warnings from collections.abc import Iterator from pathlib import Path @@ -59,13 +60,6 @@ re.VERBOSE, ) -# Pristine copies of colorama's process-wide color singletons, taken at import time -# and therefore before any Outputter can blank them. An Outputter that asks for color -# restores from these, so a no-color Outputter that was never used as a context -# manager cannot leave the process permanently colorless. -_pristine_fore = dict(Fore.__dict__) -_pristine_style = dict(Style.__dict__) - class Outputter: """Handler for print statements and saving to text and/or csv files.""" @@ -121,29 +115,12 @@ def __init__( else: self._column_widths = tuple(default_widths) - # When color is turned off, colorama's process-wide `Fore`/`Style` - # singletons are blanked so that (a) no ANSI codes are emitted and - # (b) column alignment is computed on codeless strings — `side_by_side` - # pads the leading gutter by `len(Fore.RED)` to compensate for escape - # sequences, and blanking makes that compensation zero. Because those - # singletons are global, the mutation is bounded from both ends: the - # previous values are saved here and restored in `__exit__`, and asking - # for color restores the pristine values, which repairs the state even - # if some earlier Outputter was never used as a context manager. - self._saved_fore: dict | None = None - self._saved_style: dict | None = None - if no_color: - # Replace colorized styles with blank strings. - self._saved_fore = dict(Fore.__dict__) - self._saved_style = dict(Style.__dict__) - for k in list(Fore.__dict__): - Fore.__dict__[k] = "" - for k in list(Style.__dict__): - Style.__dict__[k] = "" - else: - Fore.__dict__.update(_pristine_fore) - Style.__dict__.update(_pristine_style) - colorama.init(autoreset=True) + # Keep the palette local so interleaved comparisons can disagree about color. + self._no_color = no_color + self._red = "" if no_color else Fore.RED + self._cyan = "" if no_color else Fore.CYAN + self._heading = "" if no_color else Fore.LIGHTBLUE_EX + self._normal = "" if no_color else Fore.WHITE + Style.RESET_ALL # Open a file (this overwrites any existing file at this path). if text_file: @@ -158,12 +135,6 @@ def __enter__(self): # noqa: D105 def __exit__(self, exc_type, exc_value, exc_traceback): # noqa: D105 if self._text_file_obj: self._text_file_obj.close() - # Restore the global colorama state that was blanked for no-color output, - # so color settings don't leak to later Outputters or other libraries. - if self._saved_fore is not None: - Fore.__dict__.update(self._saved_fore) - if self._saved_style is not None: - Style.__dict__.update(self._saved_style) @property def column_widths(self) -> tuple: @@ -194,6 +165,14 @@ def print( else: text_to_print = string + if self._no_color: + text_to_print = ansi_escape.sub("", text_to_print) + elif print_args.get("file") is None: + # Preserve terminal conversion, redirected-output stripping and autoreset + # without colorama.init() replacing the process-wide stdout/stderr. Built + # per write so that re-reading sys.stdout honors capsys and redirect_stdout. + print_args["file"] = colorama.AnsiToWin32(sys.stdout, autoreset=True).stream + # Execute the print command. print(text_to_print, **print_args) @@ -234,10 +213,13 @@ def _parse_single_str(s): # pylint: disable=invalid-name if self._keep_print_history: self._line_history.append(parsed_strings) - @staticmethod - def _make_normal(string): - """Return text with normal color and style.""" - return Fore.WHITE + Style.RESET_ALL + str(string) + def _make_normal(self, string): + """Return text with this Outputter's normal color and style.""" + return self._normal + str(string) + + def print_header(self, string: str) -> None: + """Print and record a section heading using this Outputter's palette.""" + self.print(self._heading + string, add_to_history=True) def side_by_side( self, @@ -280,13 +262,13 @@ def side_by_side( # If the 'b' and 'c' strings are different (or force_color is set), # then change the font of 'a' to the color red. if (highlight_diff and are_different) or (force_color is not None): - default_color = Fore.RED - if force_color is not None: - str_a = force_color + str_a - else: - str_a = default_color + str_a + color = self._red if force_color is None else force_color + if self._no_color: + color = "" + str_a = color + str_a colors = False - extra_style_space = " " * len(default_color) + # ANSI bytes occupy no display columns but count toward string padding. + extra_style_space = " " * len(color) str_marker = self._difference_marker else: colors = True @@ -375,11 +357,11 @@ def lists_diff( # Display the comparison result if contents_are_same: - msg = "\t" + Fore.CYAN + f"Are all items the same? ---> {str(contents_are_same)}." + msg = "\t" + self._cyan + f"Are all items the same? ---> {str(contents_are_same)}." if len(set_a) > 0: self.print(msg, add_to_history=True) - self.print("\t" + Fore.CYAN + str(sorted(set_a))) + self.print("\t" + self._cyan + str(sorted(set_a))) else: self.print(msg + " (No items exist.)", add_to_history=True) return 0, 0, len(list_a) @@ -387,15 +369,13 @@ def lists_diff( # If contents are different, continue... left, right, shared = count_diffs(list_a, list_b) self.print( - "\t" + "Are all items the same? ---> " + Fore.RED + f"{str(contents_are_same)}." + "\t" + "Are all items the same? ---> " + self._red + f"{str(contents_are_same)}." f" ({_item_is_or_are(shared)} shared, out of {len(s_union)} total.)", add_to_history=True, ) # Which variables are different? - self.print("\t" + Fore.RED + "Which items are different?") - # print(Fore.RED + "Which items are different? ---> %s." % - # str(set(list_a).symmetric_difference(list_b))) + self.print("\t" + self._red + "Which items are different?") self.side_by_side(" ", "File A", "File B") self.side_by_side_list_diff(list_a, list_b) diff --git a/tests/test_printing.py b/tests/test_printing.py index 108ddee..b81d37e 100644 --- a/tests/test_printing.py +++ b/tests/test_printing.py @@ -24,10 +24,15 @@ # See the License for the specific language governing permissions and limitations under the License. +import sys +from concurrent.futures import ThreadPoolExecutor +from io import StringIO +from threading import Barrier + import pytest from colorama import Fore, Style -from ncompare.printing import Outputter +from ncompare.printing import Outputter, ansi_escape def test_list_of_strings_diff(outputter_to_console): @@ -52,39 +57,81 @@ def test_add_to_history_records_one_row_with_ansi_and_newlines_stripped(): assert out._line_history == [["red", "plain", "42"]] -def test_no_color_state_is_restored_after_context_exit(): - """`no_color=True` must not permanently blank colorama's global Fore/Style. - - Regression test: previously the color singletons were blanked and never - restored, so a no-color comparison left later comparisons (and any other - in-process colorama users) colorless. - """ - original_red = Fore.RED - original_reset = Style.RESET_ALL - assert original_red != "" # sanity check: colors are present to begin with - - with Outputter(no_color=True): - # Colors are stripped while the no-color Outputter is active. - assert Fore.RED == "" - assert Style.RESET_ALL == "" - - # ...and restored on exit, so subsequent output can be colorized again. - assert Fore.RED == original_red - assert Style.RESET_ALL == original_reset - - -def test_no_color_leak_is_repaired_by_a_later_colorized_outputter(): - """A no-color Outputter used without `with` cannot leave colorama colorless for good. - - `__exit__` is the usual restore point, but nothing obliges a caller to use the - context manager, so a colorized Outputter also repairs the global state when it - is constructed. - """ - original_red = Fore.RED - assert original_red != "" # sanity check: colors are present to begin with +class TerminalBuffer(StringIO): + """Capture ANSI output without colorama treating the stream as a pipe.""" + + def isatty(self): + return True + + +@pytest.mark.parametrize("plain_first", [False, True]) +def test_interleaved_outputters_keep_independent_colors(monkeypatch, plain_first): + """The last constructed Outputter must not change the other one's palette.""" + terminal = TerminalBuffer() + monkeypatch.setattr(sys, "stdout", terminal) + first = Outputter(no_color=plain_first, keep_print_history=True) + second = Outputter(no_color=not plain_first, keep_print_history=True) + outputs = {} + for out, no_color in [(first, plain_first), (second, not plain_first), (first, plain_first)]: + terminal.seek(0) + terminal.truncate(0) + out.print_header("\nSection heading") + out.lists_diff(["shared", "left"], ["shared", "right"]) + out.side_by_side("forced", "a", "b", force_color=Fore.LIGHTBLUE_EX) + text = terminal.getvalue() + assert ("\x1b[" not in text) == no_color + outputs[no_color] = text + assert ansi_escape.sub("", outputs[False]) == outputs[True] + assert all( + "\x1b[" not in cell for out in [first, second] for row in out._line_history for cell in row + ) - Outputter(no_color=True) # deliberately not a context manager: `__exit__` never runs - assert Fore.RED == "" - Outputter() # asking for color repairs whatever the previous Outputter left behind - assert Fore.RED == original_red +def test_no_color_does_not_mutate_colorama_even_on_exception(): + """Color constants remain usable by other libraries throughout the context.""" + original_fore, original_style = dict(Fore.__dict__), dict(Style.__dict__) + with pytest.raises(RuntimeError, match="test exit"): + with Outputter(no_color=True): + assert Fore.__dict__ == original_fore + assert Style.__dict__ == original_style + raise RuntimeError("test exit") + assert Fore.__dict__ == original_fore + assert Style.__dict__ == original_style + + +def test_parallel_outputters_keep_independent_colors(): + """Synchronize construction to expose interference between opposite palettes.""" + barrier = Barrier(2) + + def render(no_color): + out = Outputter(no_color=no_color) + stream = StringIO() + barrier.wait(timeout=5) + out.print("parallel report", file=stream) + return stream.getvalue() + + with ThreadPoolExecutor(max_workers=2) as pool: + colored, plain = list(pool.map(render, [False, True])) + assert "\x1b[" in colored + assert "\x1b[" not in plain + assert ansi_escape.sub("", colored) == plain + + +def test_no_color_strips_explicit_styles(): + """Explicitly styled input also obeys the Outputter's no-color setting.""" + stream = StringIO() + with Outputter(no_color=True) as out: + out.print("\x1b[31mred\x1b[0m", colors=True, file=stream) + assert stream.getvalue() == "red\n" + + +@pytest.mark.parametrize("no_color", [False, True]) +def test_outputter_does_not_replace_standard_streams(no_color): + """Rendering must not install global colorama wrappers on caller streams.""" + stdout, stderr = sys.stdout, sys.stderr + with Outputter(no_color=no_color) as out: + out.print_header("heading") + assert sys.stdout is stdout + assert sys.stderr is stderr + assert sys.stdout is stdout + assert sys.stderr is stderr