Skip to content

Fix #2114 - Cleanup the test folder + automation #2143

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

Merged
merged 4 commits into from
Aug 8, 2024

Conversation

WebReflection
Copy link
Contributor

@WebReflection WebReflection commented Aug 8, 2024

Description

This MR reorganized tests currently present in our test and tests folder + it creates automatically a tests/index.html page that makes it easier to manually test things around.

A lot of tests require manual or visual verification and I didn't bother removing those handy one-off pages which I do, in fact, use pretty often to validate or check things.

I hope this initial cleanup would help us in the present/future to keep the test folder clean and well organized, by also trying to not create single .html files and put everything in folders when necessary.

Changes

  • automate tests/index.html creation like it's done in polyscript
  • re-organized the whole folder structure into tests/ folder
  • moved test/integration into tests/js-integration and updated tests using the test reference in pytest to use now directly tests instead
  • checked that everything works as expected and all relative links per each test work just fine too

Checklist

  • All tests pass locally
  • I have updated CHANGELOG.md
  • I have created documentation for this(if applicable)

@WebReflection WebReflection requested a review from ntoll August 8, 2024 10:12
Copy link
Member

@ntoll ntoll left a comment

Choose a reason for hiding this comment

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

Hmm... we have test and tests. 🤣

@WebReflection
Copy link
Contributor Author

@ntoll that's always been the case ... but tests has only Python content in it and it's everything but trivial to test for real with a browser ... to be honest I consider that folder for Python tests or Python code while everything running on the front end as JS should imho be easy to verify and check. I wouldn't mind renaming either folder to make things a bit more clear ... such as py-tests for the current tests and js-tests for the others ... would that make more sense to you?

@WebReflection
Copy link
Contributor Author

@ntoll to be honest, there is a lot going on in the tests folder and it's fully handled by pytest ... I don't think it would be great to merge the two folders together but as most things are just inside tests/integration maybe I can try to move things around and just have a single tests folder?

@WebReflection
Copy link
Contributor Author

@ntoll OK ... I've merged everything into tests ... if CI is happy, I am happy too and we can move forward, I hope that's OK to you too.

@ntoll
Copy link
Member

ntoll commented Aug 8, 2024

Works for me.

@WebReflection
Copy link
Contributor Author

Works for me.

not for CI though ... the error in pyscript_dom is super weird ... I have tested manually and everything looks fine? maybe we had some test relying on that test folder first ... investigating.

@WebReflection
Copy link
Contributor Author

FYI ... a classic example on why I prefer real browsers and folders with real Web code in there and nothing else ... the pyscript_dom test was based on fake server something and impossible to debug or test, everything has been moved as part of the js-integration test now. CI should go green now and I'll merge after.

@WebReflection WebReflection force-pushed the issue-2114 branch 3 times, most recently from 8d3e18c to 3021798 Compare August 8, 2024 14:37
@WebReflection
Copy link
Contributor Author

OK, I think this is fine now ... it should pass. Basically in pytest we were trusting and using the test folder (not tests) to clone or point stuff at that. I've put back py tests where they were but updated all references to test, now pointing at tests instead. Once this goes green I'll merge but surely we need to create some docs, at some point, to better explain the current state and tests tree to make sense out of it.

@WebReflection WebReflection merged commit 9f46234 into pyscript:main Aug 8, 2024
2 checks passed
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