Skip to content

TST: Adjustments for my pcdswidgets PR#168

Merged
ZLLentz merged 5 commits intomasterfrom
tst_from_installed
Apr 10, 2026
Merged

TST: Adjustments for my pcdswidgets PR#168
ZLLentz merged 5 commits intomasterfrom
tst_from_installed

Conversation

@ZLLentz
Copy link
Copy Markdown
Member

@ZLLentz ZLLentz commented Mar 12, 2026

This makes my pcdswidgets tests pass and helps the docs build properly

See pcdshub/pcdswidgets#96

Change 1: run conda test from installed version

  • I've added a build step to pcdswidgets
  • This builds and installs correctly
  • Without this change, the conda tests run on the pre-built version of the package

Things to consider:

  • This makes the test output pretty awful (micromamba/conda-bld/pcdswidgets_1773337587302/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib/python3.12/site-packages/pcdswidgets/tests/test_builder.py::test_it_was_built[ui_source0] PASSED [ 1%])
  • This ensures that we're testing on the version of the package that gets installed
  • I didn't change the pypi tests because the pypi build runs and builds install-time files in the cloned repo, so the tests work for me, but maybe this should also be done on the pypi tests.

Change 2: expose the docs extras argument

  • In pcdswidgets I need to keep pyqt out of the pypi requirements for smooth local environment generation
  • I then need to add it back for the pip tests and docs builds
  • There is a hook for the docs builds that we hadn't been exposing, this exposes it

@ZLLentz ZLLentz changed the title TST: run conda test from installed version TST: Adjustments for my pcdswidgets PR Mar 25, 2026
Copy link
Copy Markdown
Contributor

@tangkong tangkong left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me

@ZLLentz
Copy link
Copy Markdown
Member Author

ZLLentz commented Apr 7, 2026

Is it worth keeping the build changes in if I end up removing the extra build step in pcdswidgets? I guess that's not a sure thing yet

@tangkong
Copy link
Copy Markdown
Contributor

tangkong commented Apr 7, 2026

I suppose I don't expect this to really change our current CI processes and opens up more options for us, so it's still a net positive? We can decide when pcdswidgets is in a more stable place I suppose

@ZLLentz
Copy link
Copy Markdown
Member Author

ZLLentz commented Apr 7, 2026

I'm mainly 50/50 because of the ergonomics of

bld/pcdswidgets_1773337587302/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib/python3.12/site-packages/pcdswidgets/tests/test_builder.py::test_it_was_built[ui_source0] PASSED [  1%]

@ZLLentz
Copy link
Copy Markdown
Member Author

ZLLentz commented Apr 10, 2026

I'm going to merge this in and then if later I decide I don't like placehold_placehold_placehold we'll address that some other way- there must be a way to make conda use like 2 placeholds instead of 50

@ZLLentz ZLLentz marked this pull request as ready for review April 10, 2026 21:20
@ZLLentz ZLLentz requested a review from a team as a code owner April 10, 2026 21:20
@ZLLentz
Copy link
Copy Markdown
Member Author

ZLLentz commented Apr 10, 2026

Geez apparently the long placeholder path is intentional to do some binary string replacement shenanigans, wild stuff

@ZLLentz
Copy link
Copy Markdown
Member Author

ZLLentz commented Apr 10, 2026

The correct fix for placeholder spam is to actually install the built package instead of running from the env created for the build- but maybe this will never be needed

@ZLLentz ZLLentz merged commit d61a293 into master Apr 10, 2026
5 checks passed
@ZLLentz ZLLentz deleted the tst_from_installed branch April 10, 2026 21:22
ZLLentz added a commit that referenced this pull request Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants