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

Skip import/export tests if pulpcore 3.39 #1735

Merged
merged 2 commits into from Nov 27, 2023

Conversation

pcreech
Copy link
Member

@pcreech pcreech commented Nov 27, 2023

No description provided.

@ianballou
Copy link
Contributor

I think the ostree ones were actually passing but failing silently. The export tests seemed to be the actual failure points https://ci.theforeman.org/blue/rest/organizations/jenkins/pipelines/katello-nightly-rpm-pipeline/runs/1838/log/?start=0

@pcreech
Copy link
Member Author

pcreech commented Nov 27, 2023

ack, moving around. I can probably go ahead and handle the .gz failures as well. what's the new extension?

@dralley
Copy link
Contributor

dralley commented Nov 27, 2023

It's just .tar, rather than .tar.gz

@pcreech
Copy link
Member Author

pcreech commented Nov 27, 2023

Updated, added second commit switching .gz to .tar

@evgeni
Copy link
Member

evgeni commented Nov 27, 2023

Updated, added second commit switching .gz to .tar

This will break for all releases using versions before 3.39, can we use *.tar*, assuming the old ones are .tar.gz?

@evgeni
Copy link
Member

evgeni commented Nov 27, 2023

Also, doesn't this disableall Export/Import tests?

@pcreech pcreech changed the title Skip ostree tests if pulpcore 3.39 Skip import/export tests if pulpcore 3.39 Nov 27, 2023
@pcreech
Copy link
Member Author

pcreech commented Nov 27, 2023

It does disable all, it appeared the issue was in a few. Is the suggestion to keep incremental in?

@evgeni
Copy link
Member

evgeni commented Nov 27, 2023

Ah, I think I've read the original comment wrongly. The idea is to disable all import/export tests for 3.39 until that one Pulpcore big is fixed?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

At least the function name needs to be fixed.

hammer content-export complete version --organization="${ORGANIZATION}" \
--content-view="${CONTENT_VIEW}" --version="1.0"
export_version_id=$(hammer --output csv --no-headers content-view version show --version="1.0" --content-view="${CONTENT_VIEW}" --organization="${ORGANIZATION}" \
--fields=id)
actual_size=$(du -k "$(hammer --output csv --no-headers content-export list --content-view-version-id=$export_version_id --fields="path")"/*.gz | cut -f 1)
actual_size=$(du -k "$(hammer --output csv --no-headers content-export list --content-view-version-id=$export_version_id --fields="path")"/*.tar | cut -f 1)

[ $actual_size -ge 40 ]
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but can we actually list the contents of the tar file and check that? It would also validate the resulting file(s) are actually valid tar files.

As for the rename: don't we need to keep the gzip bits for older Pulp versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on evgeni's suggestion, and verifying with dalley that it is so, I've switched it to do ".tar*' to encapsulate gz without needing to resort to adding complexity with if/else logic

@@ -28,6 +28,12 @@ tKatelloVersion() {
) | cut -d. -f1-2
}

tSkipIfPulp39() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
tSkipIfPulp39() {
tSkipIfPulp339() {

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks Visual Studio Code! Or, more specifically, the need to save files in editors.... :D

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be fixed now. Ctl+s to the rescue!

@pcreech
Copy link
Member Author

pcreech commented Nov 27, 2023

@ekohl @evgeni UPdated

Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

It looks as nice as such a hack can be :)

@pcreech pcreech merged commit 65b68c4 into theforeman:master Nov 27, 2023
8 checks passed
@pcreech pcreech deleted the skip-ostre-pulp339 branch November 27, 2023 19:30
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.

None yet

5 participants