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

No need to configure build for tests #324

Conversation

lachmanfrantisek
Copy link
Member

@lachmanfrantisek lachmanfrantisek commented Jan 13, 2020

  • Run the build for the test job as well.
  • Restructure the handler/helper classes code for build/test
    • Rename CoprBuildHandler to CoprBuildJobHelper because it's not related to the Handler classes in the code.
    • Create TestingFarmJobHelper and share the code with the common superclass of the CoprBuildJobHelper.
    • Improve CoprBuildJobHelper to not need the copr_build job and calculate the right chroots.
      • Fix the aliases for the testing farm.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@lachmanfrantisek lachmanfrantisek changed the title WIP: no need to configure build for tests No need to configure build for tests Jan 14, 2020
@lachmanfrantisek lachmanfrantisek added the ready-for-review Pull request is ready for review. label Jan 14, 2020
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

Copy link
Member

@TomasTomecek TomasTomecek left a comment

Choose a reason for hiding this comment

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

Let's ship it to stage!

The first commit is impossible to review, the rest looks great!

),
],
)
def test_targets(jobs, build_targets, test_targets):
Copy link
Member

Choose a reason for hiding this comment

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

these test cases <3

Comment on lines +299 to +301
is_copr_build: Callable[
[JobConfig], bool
] = lambda job: job.type == JobType.copr_build

if self.job.job == JobType.tests and any(
filter(is_copr_build, self.package_config.jobs)
):
Copy link
Member

Choose a reason for hiding this comment

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

I'd write this with a for loop, or at least pass the lambda directly to the filter call

but if you and Jirka think this is fine, let's roll

Copy link
Member Author

Choose a reason for hiding this comment

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

It's (currently) impossible to type lambda directly... I would be glad to see a better way to satisfy mypy...

Signed-off-by: Frantisek Lachman <flachman@redhat.com>
Signed-off-by: Frantisek Lachman <flachman@redhat.com>
Signed-off-by: Frantisek Lachman <flachman@redhat.com>
Signed-off-by: Frantisek Lachman <flachman@redhat.com>
Signed-off-by: Frantisek Lachman <flachman@redhat.com>
Signed-off-by: Frantisek Lachman <flachman@redhat.com>
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@lachmanfrantisek lachmanfrantisek added mergeit When set, zuul wil gate and merge the PR. and removed ready-for-review Pull request is ready for review. labels Jan 15, 2020
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit fecb51e into packit:master Jan 15, 2020
@lachmanfrantisek lachmanfrantisek deleted the no-need-for-build-for-tests branch January 15, 2020 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants