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

Add CheckConsoleText(WIP) #3868

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Add CheckConsoleText(WIP) #3868

wants to merge 8 commits into from

Conversation

c4rt0
Copy link
Member

@c4rt0 c4rt0 commented Sep 2, 2024

Ref: #3788

@c4rt0 c4rt0 self-assigned this Sep 2, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Sep 2, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@c4rt0 c4rt0 changed the title Add CheckConsoleText, allowing discussion Add CheckConsoleText(WIP) Sep 2, 2024
@c4rt0
Track progress - modify go routine
1e17778 
Since I had a problem with localizing the consolefile I changed the
code slightly to present contents of a /tmp/kola directory.
I then wrote a simple logic to eliminate errors and
output the content of a file for testing.

This will certainly be squashed, and cleaned up - for now uploading
just so I can somewhat track my progress here.
@c4rt0
Copy link
Member Author

c4rt0 commented Sep 24, 2024

As I work on this issue I noticed that awaitCompletion() does not specify what console is. In this case (if my understanding is not flawed) Go automatically assigns it false hence forces the code to execute every time. NewBaseQemuBuilder() assigns it through builder.InheritConsole ... I wonder if that's a bug in case of awaitCompletion?

@jbtrystram
Copy link
Contributor

Running the tests with --console should set the value to true

@c4rt0
Copy link
Member Author

c4rt0 commented Sep 24, 2024

Running the tests with --console should set the value to true

Now, that makes sense :) Merci!

@c4rt0
Copy link
Member Author

c4rt0 commented Sep 30, 2024

Just a quick update:
Some things in this PR are most likely not-recommend.

  1. hardcoding the console.txt
    consoleFilePath := filepath.Join(outdir, "console.txt")
    Especially, since platform.Machine already contains ConsoleOutput
    Moving to consoleFilePath := m.ConsoleOutput() instead.

  2. passing m platform.Machine into all of the testiso tests (don't know how I came up with that idea)
    ie: func testLiveFIPS(ctx context.Context, outdir string, m platform.Machine)

Will work on the above and push once completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@c4rt0 c4rt0

Labels
do-not-merge/work-in-progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@c4rt0 @jbtrystram