Test mode post merge cleanups#893
Conversation
Replace custom mock_context fixture with the shared tmp_context fixture from conftest.py, using a real WorkContext instead of a mock. Closes python-wheel-build#892 Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
LalatenduMohanty
force-pushed
the
test-mode-cleanup-1
branch
3 times, most recently
from
1c5640d to
1dafd38
Compare
| else: | ||
| version = None | ||
| self._record_test_mode_failure(req, version, err, "bootstrap") | ||
| logger.info(f"bootstrapping {req} as {req_type} dependency of {self.why[-1:]}") |
There was a problem hiding this comment.
The resolver logic needs to be inside the try/except, since it is still a common source of failures.
There was a problem hiding this comment.
This is a good suggestion. Will fix it.
src/fromager/bootstrapper.py
Outdated
There was a problem hiding this comment.
Maybe this logic could move to bootstrap too? Then this internal function becomes the "actually do the bootstrap" function and the outer one is managing the setup and error handling.
There was a problem hiding this comment.
@dhellmann Do you mean the code for cyclic dependencies and redundant processing logic should move from _bootstrap_impl to bootstrap() ?
Move resolve_version() and _track_why context manager from _bootstrap_impl to bootstrap() for cleaner exception handling and direct access to resolved_version without cache lookup. Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
LalatenduMohanty
force-pushed
the
test-mode-cleanup-1
branch
from
1dafd38 to
400544a
Compare
…strap() Cleaner separation: bootstrap() handles setup/validation/error-handling, _bootstrap_impl() performs only the actual build work. Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
LalatenduMohanty
force-pushed
the
test-mode-cleanup-1
branch
from
bbad8fa to
5bb0b55
Compare
dhellmann
left a comment
dhellmann
left a comment
There was a problem hiding this comment.
We should add an e2e test for something where bootstrapping fails, too.
@dhellmann I have created #895 to track the e2e test |
2 participants
Contains following commits
Followup clean ups suggested by @dhellmann during review of #865 . There is one more cleanup but it will be larger refactor, so planning to send another PR for that.