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

Add environment support to the discover step #145

Merged
merged 1 commit into from
Mar 17, 2020
Merged

Conversation

psss
Copy link
Collaborator

@psss psss commented Mar 5, 2020

Combine L1 and L2 environment in Test.export().
Include a simple test with an environment variable.
Extend utils.format() to support dictionaries as well.

@psss psss requested review from lukaszachy, pvalena and thrix March 5, 2020 17:16
@psss psss added step | discover Stuff related to the discover step enhancement labels Mar 5, 2020
@psss
Copy link
Collaborator Author

psss commented Mar 5, 2020

Any test with environment variable set still fails because of problem mentioned in #99.

@pvalena
Copy link
Collaborator

pvalena commented Mar 12, 2020

LGTM.

@pvalena
Copy link
Collaborator

pvalena commented Mar 12, 2020

/test

@pvalena pvalena mentioned this pull request Mar 12, 2020
Combine L1 and L2 environment in Test.export().
Include a simple test with an environment variable.
Extend utils.format() to support dictionaries as well.
@psss psss force-pushed the environment-discover branch from 37d8279 to c12e615 Compare March 16, 2020 12:59
@psss
Copy link
Collaborator Author

psss commented Mar 16, 2020

@pvalena, thanks for the review. @thrix, I had to disable the new test because of missing environment support in cruncher. Seems we will be running into similar issues more and more often. Would be nice to replace cruncher with tmt (relatively) soon.

@thrix thrix self-requested a review March 16, 2020 15:28
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

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

LGTM

@pvalena
Copy link
Collaborator

pvalena commented Mar 16, 2020

Seems we will be running into similar issues more and more often. Would be nice to replace cruncher with tmt (relatively) soon.

@psss for my information, what's blocking that (from tmt side)?

@psss
Copy link
Collaborator Author

psss commented Mar 17, 2020

/packit test

@psss
Copy link
Collaborator Author

psss commented Mar 17, 2020

@psss for my information, what's blocking that (from tmt side)?

Would be good to improve the output/logging/results presentation. But I think the main point is just the priority. The cruncher is somewhat stable and changing that would require extra time and effort. We just need to prioritize this well.

@psss
Copy link
Collaborator Author

psss commented Mar 17, 2020

/packit test

@psss psss self-assigned this Mar 17, 2020
@psss
Copy link
Collaborator Author

psss commented Mar 17, 2020

All tests passed, just the results are not updated here. Let's give it one more attempt.

@psss
Copy link
Collaborator Author

psss commented Mar 17, 2020

/packit build

@lachmanfrantisek
Copy link
Contributor

/packit test

@psss
Copy link
Collaborator Author

psss commented Mar 17, 2020

All tests passed. There is a problem with Packit results reporting. Merging.

@psss psss merged commit 10047f4 into master Mar 17, 2020
@psss psss deleted the environment-discover branch March 17, 2020 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
step | discover Stuff related to the discover step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants