Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Consolidate some legacy test constructs #2773

Closed
1 of 6 tasks
dmnks opened this issue Nov 21, 2023 · 8 comments
Closed
1 of 6 tasks

Consolidate some legacy test constructs #2773

dmnks opened this issue Nov 21, 2023 · 8 comments
Labels
cleanup test Testsuite-related

Comments

@dmnks
Copy link
Contributor

dmnks commented Nov 21, 2023

  • Merge RPMDB_INIT into RPMTEST_SETUP
  • Rename RPMTEST_SETUP to RPMTEST_INIT
  • Add RPMTEST_SETUP as a wrapper for AT_SETUP (for consistency)
  • Drop run()
  • (stretch goal) Define wrappers for the RPM binaries (rpm, rpmbuild, rpmsign, ...) which would be used instead of runroot*() (optimization: don't run a Bubblewrap container if $RPMTEST is /)
  • ...and finally eliminate the need for explicit RPMTEST_INIT at all

Thanks @pmatilai for the ideas! 😄

@dmnks
Copy link
Contributor Author

dmnks commented Nov 21, 2023

* Perhaps `RPMTEST_SETUP` isn't very descriptive, choose a better name? Its purpose is to initialize a writable tree and mount it at `$RPMTEST`

Hmm, actually this is probably a good name after all 😄 It literally says "setup RPMTEST (directory)"...

@dmnks dmnks self-assigned this Nov 21, 2023
@dmnks dmnks added the test Testsuite-related label Nov 21, 2023
@dmnks dmnks added this to RPM Nov 21, 2023
@github-project-automation github-project-automation bot moved this to Backlog in RPM Nov 21, 2023
@dmnks dmnks moved this from Backlog to Todo in RPM Nov 21, 2023
@pmatilai
Copy link
Member

pmatilai commented Nov 21, 2023

The problem with RPMTEST_SETUP name is that it seems to relate to AT_SETUP when it doesn't. Especially as we have RPMTEST_CLEANUP which relates to AT_CLEANUP, and one day there may be a reason to wrap AT_SETUP similarly.

Maybe RPMTEST_INIT?

@dmnks
Copy link
Contributor Author

dmnks commented Nov 21, 2023

Yup, good point. It could be made into an AT_SETUP wrapper and just take some kind of argument that says "I need write access to /" but that would require a lot of (even if automated) search & replace across all the test files...

In any case, RPMTEST_INIT works for me.

@pmatilai
Copy link
Member

After it's renamed, it may not be a bad idea to actually add RPMTEST_SETUP that does, for now, nothing more than wrap AT_SETUP. Just to make the whole thing more consistent.

@dmnks
Copy link
Contributor Author

dmnks commented Nov 21, 2023

OK, I've updated the description accordingly, thanks!

@dmnks dmnks removed their assignment Mar 6, 2024
@pmatilai
Copy link
Member

Yay, we got to drop run() 😄

@pmatilai
Copy link
Member

Seems this has turned into halfway rewrite by now 😂 but it'd seem strange not to include #3542 in this as well...

@pmatilai
Copy link
Member

I think we'll consider this step done now. There are other legacy constructs to deal with but those are better dealt with on in their own tickets.

@github-project-automation github-project-automation bot moved this from Todo to Done in RPM Jan 30, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cleanup test Testsuite-related
Projects
Status: Done
Development

No branches or pull requests

2 participants