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

Migrated initial prep material tests, all currently passing. #450

Merged
merged 21 commits into from
Aug 14, 2023

Conversation

TheDuckCow
Copy link
Member

Work in progress migration of unittests for MCprep to standard unittest library and the new unittest framework.

@TheDuckCow TheDuckCow linked an issue Jul 23, 2023 that may be closed by this pull request
All tests passing, but also created disabled by default tests that
have UI triggered popups which would be annoying to have on by default,
but are good for code coverage.
Base automatically changed from 424-migrate-to-the-unittest-library to milestone-3-5-0 August 6, 2023 05:45
@TheDuckCow
Copy link
Member Author

Continuing to make progress here. So far, tests are working nice and fast. Once I get to OBJ loading, I intend to set up a way to specify running 'slow/medium' tests over just fast tests.

Current results (woohoo, already some validation that blender 4.0 isn't going to break everything!)

-------------------------------------------------------------------------------
bversion   	ran_tests	ran	skips	failed	errors
-------------------------------------------------------------------------------
(3.6.0)   	all_tests	15	2	0	No errors
(4.0.0)   	all_tests	15	2	0	No errors
(3.5.1)   	all_tests	15	2	0	No errors
(3.4.0)   	all_tests	15	2	0	No errors
(3.3.1)   	all_tests	15	2	0	No errors
(3.2.1)   	all_tests	15	2	0	No errors
(3.1.0)   	all_tests	15	2	0	No errors
(3.0.0)   	all_tests	15	2	0	No errors
(2.93.0)   	all_tests	15	2	0	No errors
(2.90.1)   	all_tests	15	2	0	No errors
(2.80.75)   	all_tests	14	3	0	No errors

@TheDuckCow
Copy link
Member Author

TheDuckCow commented Aug 6, 2023

While it would be good to have all tests migrated before the 3.5.0 release, it might be good to break this PR off at some point into other pieces, particularly since it contains at least one bug fix already. My plan is to migrate at least the following absolutely critical tests, based on the most commonly performed actions:

  • spawn_mob
  • change_skin
  • import_world_split
  • model_spawner
  • world_tools (which includes add_mc_sky)

FYI @StandingPadAnimations once these are migrated I'll request your review. Once this is merged, I'd be more supportive of a "release candidate".

Not asking for your review yet, as I need to double check that all the right resources are checked in with the new test runner (and I haven't deleted the old one yet, since I'm using it as a sort of progress check, so it's probably confusing as-is right now).

Test intentionally does not yet pass, as there's a bug in the code to
fix first. Will do so in the next commit.
One reference wasn't using the new system for version-agnostic mix rgb
slot selection, that is now fixed and all tests are passing across all
versions.
All tests passing, but also created disabled by default tests that
have UI triggered popups which would be annoying to have on by default,
but are good for code coverage.
Test intentionally does not yet pass, as there's a bug in the code to
fix first. Will do so in the next commit.
One reference wasn't using the new system for version-agnostic mix rgb
slot selection, that is now fixed and all tests are passing across all
versions.
Tests are slower to run now since there's no -fast command, but all
tests currently run.
@TheDuckCow TheDuckCow marked this pull request as ready for review August 9, 2023 06:50
@TheDuckCow
Copy link
Member Author

Marking this ready to review - will do other critical tests in another branch following this one.

This change so far includes a number of bug fixes in addition to just the test suit adoption itself. @StandingPadAnimations would be great to make sure the tests all run and pass on your machine as it is, to validate it's cross platform as hoped.

-------------------------------------------------------------------------------
bversion   	ran_tests	ran	skips	failed	errors
-------------------------------------------------------------------------------
(3.6.0)   	all_tests	24	2	0	No errors
(4.0.0)   	all_tests	24	2	0	No errors
(3.5.1)   	all_tests	24	2	0	No errors
(3.4.0)   	all_tests	24	2	0	No errors
(3.3.1)   	all_tests	24	2	0	No errors
(3.2.1)   	all_tests	24	2	0	No errors
(3.1.0)   	all_tests	24	2	0	No errors
(3.0.0)   	all_tests	24	2	0	No errors
(2.93.0)   	all_tests	24	2	0	No errors
(2.90.1)   	all_tests	24	2	0	No errors
(2.80.75)   	all_tests	23	3	0	No errors
tests took 35s to run

@StandingPadAnimations
Copy link
Collaborator

Uhhh....

-------------------------------------------------------------------------------
bversion        ran_tests       ran     skips   failed  errors
-------------------------------------------------------------------------------
(3.6.0)         all_tests       24      2       17      test_download_username_list;test_generate_material_sequence;test_load_material_animated;test_prep_missing_pbr_passes;test_prep_pbr_seus;test_prep_pbr_specular;test_prep_simple;test_skin_swap_local;test_skin_swap_username;test_spawn_with_skin;test_img_sequence_effect_spawner;test_particle_plane_effect_spawner;test_item_spawner;test_item_spawner_resize;test_world_import_jmc_full;test_world_import_mineways_combined;test_world_import_mineways_separated
tests took 2s to run

@TheDuckCow
Copy link
Member Author

Welp good I asked, @StandingPadAnimations can you share the details of what failed? Feel free to dump into a text file so I can better debug what the problem is. I'll give a go at setting the repo on another machine to do further troubleshooting.

@StandingPadAnimations
Copy link
Collaborator

Here's the logs
log.txt

@TheDuckCow
Copy link
Member Author

@StandingPadAnimations Oh yeah it looks like an import error in the module that everything is decorated by, I'm amazed any tests pass.

Hard for me to troubleshoot why NameError: name 'env' is not defined in the tracking.py file for you but that's why everything is failing. Are you sure your local build system is working ok, copying over all files? I'm using the current released version of it on pip.

@TheDuckCow
Copy link
Member Author

FYI @StandingPadAnimations I just did a fresh install of bpy-addon-build and a freshly cloned MCprep repo and copy of the currently released MCprep_resources folder; with that, I'm getting all 24,2,0 for ran,skipped,failed as I am getting on my OSX machine (the two skips are expected, since they'd annoying open folders and browser tabs). I'd try doing a clean or deletion of your MCprep install locally, or the public build of bpy-adodn-build. Let me know if you still can't get it to work.

My windows machine results. Not sure what's going on in my local 2.90, and the one random failure in 3.0.0 was due to some error trying to load a Warden rig (huh), but otherwise looking like it's working as expected. I'll investigate both, but seems unrelated to your experience of env missing. Also clearly my PC has aged, definitely not quite as fast to ran as on my M1 laptop!

-------------------------------------------------------------------------------
bversion        ran_tests       ran     skips   failed  errors
-------------------------------------------------------------------------------
(3.5.0)         all_tests       24      2       0       No errors
(3.0.0)         all_tests       24      2       1       test_spawn_with_skin
(2.93.0)        all_tests       24      2       0       No errors
(2.90.0)        all_tests       24      2       7       test_detect_desaturated_images [Assert grass_block_top saturate is True];test_detect_desaturated_images [Assert acacia_leaves saturate is True];test_detect_desaturated_images [Assert redstone_dust_line0 saturate is True];test_detect_desaturated_images [Assert water_flow saturate is True];test_detect_desaturated_images [Assert grass_block_side saturate is False];test_detect_desaturated_images [Assert glowstone saturate is False];test_matprep_cycles

tests took 78s to run

@TheDuckCow
Copy link
Member Author

All "mission critical tests" have been copied over now, though many more sub functionality and edge case preserving tests are pending to be migrated over. But it shouldn't hold up merging this into the milestone branch.

Current results on my OSX machine, this includes another fix to get blender 4.0 working:

-------------------------------------------------------------------------------
bversion   	ran_tests	ran	skips	failed	errors
-------------------------------------------------------------------------------
(3.6.0)   	all_tests	29	2	0	No errors
(4.0.0)   	all_tests	29	2	0	No errors
(3.5.1)   	all_tests	29	2	0	No errors
(3.4.0)   	all_tests	29	2	0	No errors
(3.3.1)   	all_tests	29	2	0	No errors
(3.2.1)   	all_tests	29	2	0	No errors
(3.1.0)   	all_tests	29	2	0	No errors
(3.0.0)   	all_tests	29	2	0	No errors
(2.93.0)   	all_tests	29	2	0	No errors
(2.90.1)   	all_tests	29	2	0	No errors
(2.80.75)   	all_tests	28	3	0	No errors
tests took 43s to run

I'm going to cut my further development here. @StandingPadAnimations let me know if you end up getting your local run of tests working after a fresh pip install or a n virtualenv, and if you see any fishy code you'd recommend changing before I merge this.

@StandingPadAnimations
Copy link
Collaborator

I've done a clean clone, made sure bpy-build was updated to the latest hotfix, still fails. It seems like the way the tests are ran on Linux cause Blender to not properly load MCprep, I'll have to look into it

This is to make sure the latest versions of every dependency is
installed, as to reduce variables in what could be causing errors.
@StandingPadAnimations
Copy link
Collaborator

Ok looking at the test files, my best guess is that somewhere env is not being set as a global variable. Perhaps it's related to modules? I can't really tell what's going on. Printing globals() in conf.py doesn't seem to indicate anything unusual.

@TheDuckCow
Copy link
Member Author

It seems like the way the tests are ran on Linux cause Blender to not properly load MCprep

Ah so this is on your linux machine, I've tested on OSX and on Windows. I'll give a go on linux later today to see if I run into the same. Not that it should matter, but have you manually enabled and saved preferences to have MCprep enabled on that instance? And does the addon actually work in every sense, like doing a few operations?

@StandingPadAnimations
Copy link
Collaborator

Linux is my daily driver these days, so I can confirm MCprep is both manually enabled and works with normal use

@TheDuckCow
Copy link
Member Author

Yikes it took me some time (bad vm, then switch to desktop mint install that needed tons of libraries reinstalled... then home permission errors..........)

But I've got it running on linux mint. Here's my first output:

-------------------------------------------------------------------------------
bversion   	ran_tests	ran	skips	failed	errors
-------------------------------------------------------------------------------
(2.90.1)   	all_tests	29	2	6	test_generate_material_sequence;test_load_material_animated;test_prep_missing_pbr_passes;test_world_import_jmc_full;test_world_import_mineways_combined;test_world_import_mineways_separated

I then thought it was interesting that it was all to do with files that should have been added to the repo (e.g. the test world), and indeed it seems the git lfs fetch failed. I think my system is messed up in more ways than one, but I know these tests are only failing due to git lfs not being installed locally

So... @StandingPadAnimations I'm not able to replicate your linux env importing issue :/ and I now have it working independently on 3 different machines/OS's (aside from my git lfs issue above). Maybe try starting fresh with a new virtual environment and py3.7 (min version to target to support Blender 2.80).

@zNightlord would also be interesting to see if the tests are working for you. You'd need to:

  1. Check out this branch
  2. Install (possibly in a python virtual environment, but that's your call) pip install bpy-addon-build
  3. Create a blender_execs.txt file in the root of git repo, with a complete path to a blender executable (e.g. /home/***/Desktop/blender-2.90/blender on linux, it'll be a .exe for windows)
  4. Run python run_tests.py and share the outputs.

@StandingPadAnimations
Copy link
Collaborator

StandingPadAnimations commented Aug 14, 2023

Testing in Python 3.7 (using Pyenv), indeed everything works now (beyond missing files, I have no idea how to use Git LFS though). I was also able to fix the Failed to open dir (No such file or directory): /run/user/1000/gvfs/ error by installing gvfs and udiske (Keep in mind I use my own desktop environment, the average Linux user should have at least gvfs installed. I don't know why it's needed for the test scripts though).

EDIT: the path for Blender on Linux depends on how you install it. For me, it's /usr/bin/blender as I installed it on the Arch repos (and for Linux distros in general it should be a similar path if installed from their repos). It's a different path if installed from the tar archive or Snap package.

@StandingPadAnimations
Copy link
Collaborator

Yep testing with Python 3.9 (again through Pyenv), it seems to work fine. Let me see if installing 3.11 through Pyenv works (I have 3.11 installed, but that's the system Python, so perhaps there's some differences)

@StandingPadAnimations
Copy link
Collaborator

StandingPadAnimations commented Aug 14, 2023

Hmm, with a fresh Python 3.11 install I get this:

[MCPREP Error] Failed tracker module load, invalid import module:
        No module named 'requests'
[MCPREP Error] Failed tracker module load, invalid import module:
        No module named 'requests'

But it works after install requests in the virtual environment. Perhaps a standard library change.

@StandingPadAnimations
Copy link
Collaborator

StandingPadAnimations commented Aug 14, 2023

I think it's safe to say that using the system Python on Linux will cause errors. That's something we should add to CONTRIBUTING.md. I think the following will sufice:

Note if you're on Linux, it's recommended to install Pyenv and install Python through it. The system Python (the Python package on Linux distros) is designed for the OS and will cause issues with tests.

@TheDuckCow
Copy link
Member Author

TheDuckCow commented Aug 14, 2023

Oh yeah I meant to add, I was using pyenv as well - my system python was too old (3.5 I think?) and I didn't want to mess with that. Yeah I think adding pyenv would be a good tip! And yes, blender pre-packages the requests library, but stock python itself doesn't. That being said, it should be that the add tests .py should be using blender's python and thus the requests library there, so curious where it turned out you needed that.

All that being said, glad to hear everything is working - I'm going to merge this into the milestone, and then merge milestone into dev cuz at this point, there's no going back!

(edit for zNight: would still be cool to see if tests pass for your, but won't block on it)

@TheDuckCow TheDuckCow merged commit 5721a12 into milestone-3-5-0 Aug 14, 2023
@TheDuckCow TheDuckCow deleted the 424-migrate-unittests branch August 14, 2023 02:46
@TheDuckCow
Copy link
Member Author

FYI all tests are passing on 2.90 and 2.80 (only ones I had to test) on my linux machine, after following these steps to get linux lfs installed, didn't realize it actually is extra steps:

https://docs.github.com/en/repositories/working-with-files/managing-large-files/installing-git-large-file-storage?platform=linux

So yes, we'll need to update the contributing guide in a few ways. There's some outdated info there and such too, and old script files that need to be removed etc.

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.

Migrate to the unittest library
2 participants