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 Artifact Manager S3 to the managed set #3843

Merged
merged 2 commits into from
Oct 27, 2024

Conversation

darinpope
Copy link
Contributor

Add Artifact Manager S3 to the managed set

Testing done

  • LINE=2.452.x PLUGINS=artifact-manager-s3 bash local-test.sh
  • LINE=2.462.x PLUGINS=artifact-manager-s3 bash local-test.sh
  • LINE=2.479.x PLUGINS=artifact-manager-s3 bash local-test.sh
  • LINE=weekly PLUGINS=artifact-manager-s3 bash local-test.sh

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@darinpope darinpope requested a review from a team as a code owner October 26, 2024 17:13
@darinpope darinpope assigned darinpope and unassigned darinpope Oct 26, 2024
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks! Please just make sure full-test is passing after you land all of these.

@MarkEWaite MarkEWaite added the full-test Test all LTS lines in this PR and do not halt upon first error. label Oct 27, 2024
@darinpope darinpope merged commit ff92fc4 into jenkinsci:master Oct 27, 2024
797 checks passed
@darinpope darinpope deleted the add-artifact-manager-s3 branch October 27, 2024 15:53
@jglick
Copy link
Member

jglick commented Oct 28, 2024

Beware that #286 (comment) still applies: the coverage you gain by this is minimal.

@basil
Copy link
Member

basil commented Oct 28, 2024

Agreed that the coverage gains for plugins whose tests use Docker is minimal, but perhaps one day we will run BOM tests in an environment where Docker is available (just as they are within CloudBees). Even if the coverage gains are minimal, running as many test suites as possible in a single job is useful in order to do things like scope out Java upgrades (for example, to discover which plugins use test libraries that won't work on recent Java runtimes).

@jglick
Copy link
Member

jglick commented Oct 29, 2024

Yes. To be clear,

plugins whose tests use Docker

is only partly the case here: most of the interesting tests require AWS credentials; the ones which can run without AWS credentials but require Docker cover some basic “smoke test” scenarios—enough to detect critical regressions in PCT in most dependencies, but perhaps not for example the AWS SDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-test Test all LTS lines in this PR and do not halt upon first error.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants