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

test(nuejs-core): for-tests #507

Merged
merged 7 commits into from
Mar 23, 2025
Merged

test(nuejs-core): for-tests #507

merged 7 commits into from
Mar 23, 2025

Conversation

nobkd
Copy link
Collaborator

@nobkd nobkd commented Mar 12, 2025

@alexpricedev maybe you can also take a look, if this looks fine to you. I roughly oriented myself on your tests.

I can also split array funcs to n tests, so only one function is tested at a time. Would probably be more deterministic / ensure their functionality.


PS: what do you think about a template index.html for the manual browser tests and creating the needed index.htmls with compile script to dist, next to the compiled components?
EDIT: added this


Another edit: I think, the for loops are currently not re-run, if we change content of a nested item. I'm gonna take a look at that tomorrow.
For now making this WIP.

Update: I don't think there's any easy solution to this, so not going to follow this here anymore. maybe in the future.

@nobkd nobkd changed the title test(domino): nue.js for test(nuejs-core): for-tests Mar 13, 2025
@nobkd nobkd force-pushed the domino-for-tests branch from 3cd7655 to dcbcb92 Compare March 13, 2025 06:00
@nobkd nobkd marked this pull request as draft March 16, 2025 04:36
@alexpricedev
Copy link
Contributor

I need to sort my notifications out as I only just saw this when looking manually! Maybe you can DM me on Slack if you need my 👀 ?

Will take a look tomorrow 🤙

@nobkd
Copy link
Collaborator Author

nobkd commented Mar 19, 2025

Oh, no worries. I changed this to wip, because I still wanted to check some stuff.
But I can message you on slack as well, when I mention you sometime in the future :)

Copy link
Contributor

@alexpricedev alexpricedev left a comment

Choose a reason for hiding this comment

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

Super cool, nice work :) I'd approve it in its current state for sure.

@nobkd nobkd marked this pull request as ready for review March 23, 2025 01:23
@nobkd nobkd force-pushed the domino-for-tests branch from dcbcb92 to 012fb93 Compare March 23, 2025 01:37
@nobkd nobkd force-pushed the domino-for-tests branch from 012fb93 to 5071213 Compare March 23, 2025 01:39
@nobkd nobkd force-pushed the domino-for-tests branch from 0801c8c to cfd97d2 Compare March 23, 2025 01:44
@nobkd
Copy link
Collaborator Author

nobkd commented Mar 23, 2025

Nice. I changed my mind about adding more to this PR, and will merge now, since I made no significant changes after the review.

@nobkd nobkd merged commit 57d704f into nuejs:master Mar 23, 2025
4 checks passed
@nobkd nobkd deleted the domino-for-tests branch March 23, 2025 01:48
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