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 the new functionality introduced in #174 #179

Closed
oyvindhagberg opened this issue Nov 29, 2023 · 0 comments · Fixed by #182
Closed

Test the new functionality introduced in #174 #179

oyvindhagberg opened this issue Nov 29, 2023 · 0 comments · Fixed by #182
Assignees
@terjekv

Comments

@oyvindhagberg
Copy link
Contributor

  • Test the new functionality introduced in feat: Allow for filtering of output #174
    • Add commands to the testsuite to test permission network_add with a regex that contains the characters | and |! , as a regression test for this commit
    • Update testsuite-result.json (use run_testsuite_and_record.sh and look at new_testsuite_log.json, comparing the files)
  • The source command / cmdline argument must use OutputManager
    • There should be no difference in how output is handled when using the source command/argument and by typing commands interactively
  • Optionally, merge Recorder and OutputManager
@oyvindhagberg oyvindhagberg mentioned this issue Nov 29, 2023
Merged
terjekv added a commit to terjekv/mreg-cli that referenced this issue Dec 1, 2023
@terjekv
feat: Merge Recorder and OutputManager.
008a421 
This PR seeks to merge Recorder and OutputManager into a unified interface for processing commands and producing output, either to file or console. This ensures that:

  - Filtering (`|`or `|!`) works from commands called via `source`
  - Output to files is filtered as expected.

### Structural changes

  - Removes recorder.py and its class.
  - Incorporates recording into OutputManager.
  - The OutputManager handles recording (mostly) behind the scenes if we're in recording mode. Outside of log handling we don't have to call recording tools explicitly as its done as part of normal output processing.
  - Incorporates table formatting into OutputManager where it belongs.
  - Moves command processing to a shared separate function called from both `source` and the interactive loop.

### CLI changes

  - Adds commands to start and stop recording interactively.
  - Updates the prompt to show active recording if applicable.

NOTE: This PR does not pass the current tests as it fixes unioslo#181. These changes should be thourughly  reviewed and verified before new test logs are created.

Fixes unioslo#179, fixes unioslo#181.
@terjekv terjekv mentioned this issue Dec 1, 2023
Merged
terjekv added a commit to terjekv/mreg-cli that referenced this issue Dec 6, 2023
@terjekv
feat: Merge Recorder and OutputManager.
836f3bc 
This PR seeks to merge Recorder and OutputManager into a unified interface for processing commands and producing output, either to file or console. This ensures that:

  - Filtering (`|`or `|!`) works from commands called via `source`
  - Output to files is filtered as expected.

  - Removes recorder.py and its class.
  - Incorporates recording into OutputManager.
  - The OutputManager handles recording (mostly) behind the scenes if we're in recording mode. Outside of log handling we don't have to call recording tools explicitly as its done as part of normal output processing.
  - Incorporates table formatting into OutputManager where it belongs.
  - Moves command processing to a shared separate function called from both `source` and the interactive loop.

  - Adds commands to start and stop recording interactively.
  - Updates the prompt to show active recording if applicable.

NOTE: This PR does not pass the current tests as it fixes unioslo#181. These changes should be thourughly  reviewed and verified before new test logs are created.

Fixes unioslo#179, fixes unioslo#181.
pederhan pushed a commit that referenced this issue Dec 6, 2023
@terjekv
feat: Merge Recorder and OutputManager. (#182)
c37d919 
* feat: Merge Recorder and OutputManager.

This PR seeks to merge Recorder and OutputManager into a unified interface for processing commands and producing output, either to file or console. This ensures that:

  - Filtering (`|`or `|!`) works from commands called via `source`
  - Output to files is filtered as expected.

  - Removes recorder.py and its class.
  - Incorporates recording into OutputManager.
  - The OutputManager handles recording (mostly) behind the scenes if we're in recording mode. Outside of log handling we don't have to call recording tools explicitly as its done as part of normal output processing.
  - Incorporates table formatting into OutputManager where it belongs.
  - Moves command processing to a shared separate function called from both `source` and the interactive loop.

  - Adds commands to start and stop recording interactively.
  - Updates the prompt to show active recording if applicable.

NOTE: This PR does not pass the current tests as it fixes #181. These changes should be thourughly  reviewed and verified before new test logs are created.

Fixes #179, fixes #181.

* Add back a comment about shell commands called from scripts.

* Clarify comments about recording related attributes.

* Avoid reimplementing stop_recording when saving.

* Update testsuite results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@terjekv terjekv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Merge Recorder and OutputManager. terjekv/mreg-cli
2 participants
@terjekv @oyvindhagberg