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

Fix a slew of new CI pipeline / test suite problems that appear after new years #2537

Merged
merged 6 commits into from
Jan 4, 2023

Conversation

lunkwill42
Copy link
Member

@lunkwill42 lunkwill42 commented Jan 3, 2023

Several changes in upstream dependencies caused our test suite to break down over X-mas vacation, making the first workday of 2023 a pain.

This resolves multiple of these issues and cleans up a few things along the way.

Since these problems will also affect the current stable branch, this PR is against the stable branch, which should be merged to master as soon as the PR is approved.

Tox

Tox 4 breaks compatibility with several bits of our tox.ini (and they must have had a lot of problems with this release, they've had 23 new releases in the month that has passed since the release of 4.0.0): https://pypi.org/project/tox/#history

Primarily, this bites us hard, in a non-compatible way: tox-dev/tox#2635

Also, the passenv option seems to require comma-separated arguments now, rather than the space-separated ones required in version 3.

We haven't supported Python 2 for quite a while now.
ciscoconfparse is a dependency of NAV's direct dependency
napalm. However, the latest few versions of ciscoconfparse have been
packaged with dependencies that are only relevant for developers, not
for production use. These dependencies conflict with our test
environment dependencies.

Until we upgrade to Napalm 4 (which ditches ciscoconfparse), we lock it
to an older, stable version during test runs.
This should get rid of all the intermittent access errors from
coverage. These are mainly due to the fact that certain subprocesses
during test setup (such as database initialization inside test
container) will run as root or other users that do not have write access
to the source root directory.

This points coverage to use a coverage database file in the reports
directory, and ensures this is world-writeable (!) so all subprocesses
can access it.
Apparently, pip<21.3 behaves differently on Python 3.7 and 3.9. The
package `crispy-forms-foundation` does not get its package data
installed (i.e. its Django templates directory) under Python 3.7, but it
does under Python 3.9.

Since the latest pip appears to work fine for NAV now, we can safely
update.
Tox 4 was released last month, and has since released 23 patch
releases. All of them break compatibility with our tox.ini in two
important regards:

`passenv` is no longer a space separated glob list, it seem it needs to
be a comma-separated one.

More importantly, tox no longer interprets quotes in commands - it
interprets them literally and feeds them as verbatim arguments to the
called commands, causing every command in our tox.ini file that uses
quotes to fail.
@github-actions
Copy link

github-actions bot commented Jan 3, 2023

Test results

     12 files       12 suites   10m 56s ⏱️
3 107 tests 3 011 ✔️   96 💤 0
8 796 runs  8 508 ✔️ 288 💤 0

Results for commit 3564982.

♻️ This comment has been updated with latest results.

It seems the upstream `ubuntu-latest` action runner was updated by
GitHub 4 days ago. The latest ubuntu is now 22.04, which features
libsnmp40 rather than libsnmp35.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Comment on lines +51 to +52
mkdir -p {toxinidir}/reports/coverage
chmod 777 {toxinidir}/reports/coverage
Copy link
Contributor

Choose a reason for hiding this comment

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

Were these automatically created before or have they been manually moved?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, only the {toxinidir}/reports part is typically created as part of a test run from before. This adds the extra subdirectory coverage, and tells coverage to put its coverage database files (primarily an SQLite3 file called .coverage) there, while making sure all subprocesses can write to it.

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

See comments re. location of coverage reports

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #2537 (3564982) into 5.5.x (fa5425a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            5.5.x    #2537   +/-   ##
=======================================
  Coverage   53.15%   53.15%           
=======================================
  Files         554      554           
  Lines       40309    40309           
=======================================
  Hits        21426    21426           
  Misses      18883    18883           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lunkwill42 lunkwill42 requested a review from stveit January 3, 2023 14:41
Copy link
Contributor

@johannaengland johannaengland left a comment

Choose a reason for hiding this comment

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

We shouldn't forget to eventually upgrade napalm, I will make an issue for that.

@lunkwill42 lunkwill42 merged commit b1fa5aa into Uninett:5.5.x Jan 4, 2023
@lunkwill42 lunkwill42 deleted the test/fix-broken-pipelines branch January 4, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants