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

unit test improvement/fixes #2923

Merged
merged 5 commits into from
Oct 14, 2020
Merged

Conversation

martinpitt
Copy link
Contributor

No description provided.

@martinpitt
Copy link
Contributor Author

As the unit tests currently fail due to a change in rawhide(first seen in PR #2922), this provides a good opportunity to validate that the log cat works as intended. 😁

@martinpitt martinpitt added port to RHEL8 master Please, use the `f39` label instead. labels Oct 14, 2020
@martinpitt
Copy link
Contributor Author

rpm-tests failure is https://bugzilla.redhat.com/show_bug.cgi?id=1887969 FTR

@martinpitt
Copy link
Contributor Author

Yes, so we can now see the failure directly in the output, and have a convenient way to link to it: https://github.com/rhinstaller/anaconda/pull/2923/checks?check_run_id=1251662037#step:6:1485

martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
pylint hits 2 GiB per process pretty often [1], thus still causing OOM
failures [2]. Bump the divisor.

[1] pylint-dev/pylint#1495
[2] pylint-dev/pylint#3899

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
With the variable reorg in commit a452f1b, `CI_TASKS_TEST_ARGS`
already contains the volume mount and `--rm`, so it's not necessary any
more to repeat it -- in fact, that fails:

    Error: /anaconda: duplicate mount destination

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
max() was bogus -- we want *both* RAM and number of CPUs to limit the
number of jobs, not take whichever is highest.

[1] pylint-dev/pylint#1495
[2] pylint-dev/pylint#3899

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
With the variable reorg in commit a452f1b, `CI_TASKS_TEST_ARGS`
already contains the volume mount and `--rm`, so it's not necessary any
more to repeat it -- in fact, that fails:

    Error: /anaconda: duplicate mount destination

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
max() was bogus -- we want *both* RAM and number of CPUs to limit the
number of jobs, not take whichever is highest.

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
With the variable reorg in commit a452f1b, `CI_TASKS_TEST_ARGS`
already contains the volume mount and `--rm`, so it's not necessary any
more to repeat it -- in fact, that fails:

    Error: /anaconda: duplicate mount destination

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
We want *both* RAM and number of CPUs to limit the number of jobs, not
take whichever is highest, and we want *at least* one job. So flip
max/min around to actually make sense.

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
With the variable reorg in commit a452f1b, `CI_TASKS_TEST_ARGS`
already contains the volume mount and `--rm`, so it's not necessary any
more to repeat it -- in fact, that fails:

    Error: /anaconda: duplicate mount destination

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
pylint hits 2 GiB per process pretty often [1], thus still causing OOM
failures [2]. Bump the divisor.

[1] pylint-dev/pylint#1495
[2] pylint-dev/pylint#3899

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
martinpitt added a commit to martinpitt/anaconda that referenced this pull request Oct 14, 2020
With the variable reorg in commit a452f1b, `CI_TASKS_TEST_ARGS`
already contains the volume mount and `--rm`, so it's not necessary any
more to repeat it -- in fact, that fails:

    Error: /anaconda: duplicate mount destination

Cherry-picked from master PR rhinstaller#2923

Related: rhbz#1885635
We want *both* RAM and number of CPUs to limit the number of jobs, not
take whichever is highest, and we want *at least* one job. So flip
max/min around to actually make sense.
pylint hits 2 GiB per process pretty often [1], thus still causing OOM
failures [2]. Bump the divisor.

[1] pylint-dev/pylint#1495
[2] pylint-dev/pylint#3899
It's more convenient to look at it directly on the workflow run page
than downloading and unpacking logs.zip.
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Thank you!

@martinpitt
Copy link
Contributor Author

Ported to rhel-8 branch in PR #2920

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

We have to find the correct solution first.

Other than that looks good to me.

.github/workflows/validate.yml Show resolved Hide resolved
tests/README.rst Show resolved Hide resolved
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks great now. Thanks!

With the variable reorg in commit a452f1b, `CI_TASKS_TEST_ARGS`
already contains the volume mount and `--rm`, so it's not necessary any
more to repeat it -- in fact, that fails:

    Error: /anaconda: duplicate mount destination
@martinpitt
Copy link
Contributor Author

I put back the "Fix ci-tasks container shell instructions" commit as per discussion with @jkonecny12 , and we'll fix the "run single test" separately in #2925.

@martinpitt martinpitt merged commit fec4ee9 into rhinstaller:master Oct 14, 2020
@martinpitt martinpitt deleted the unit-tests branch October 14, 2020 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master Please, use the `f39` label instead.
Development

Successfully merging this pull request may close these issues.

4 participants