required children prop

[siner308]

children prop cannot be REQUIRED prop now.
This PR makes it possible to set required to children prop.

Contributor Checklist

• I have broken down my PR scope into the following TODO tasks
  • Reformat old codes that broken at flake8 test
  • Make children prop checkable in __init__ function of python component
• I have run the tests locally and they passed. (refer to testing section in contributing)
• I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

[alstn2468]

LGTM

[alstn2468]

@alexcjohnson I want to add test case to test_base_component.py, but is there a specific rule for test case name such as debcXXX?

[alexcjohnson]

@alexcjohnson I want to add test case to test_base_component.py, but is there a specific rule for test case name such as debcXXX?

Thanks @alstn2468! Yes, across dash the test case names are generally two characters for the folder (development -> "de") then two characters for the filename (test_base_component -> "bc") then a 3-digit serial number. So if you call your test test_debc030_whatever you can run it solo with pytest -k debc030

That said, given that this involves the generator, it might be better to make a new component with required children in @plotly/dash-test-components, and test it in maybe tests/integration/test_generation?

[siner308]

@alexcjohnson I want to add test case to test_base_component.py, but is there a specific rule for test case name such as debcXXX?

Thanks @alstn2468! Yes, across dash the test case names are generally two characters for the folder (development -> "de") then two characters for the filename (test_base_component -> "bc") then a 3-digit serial number. So if you call your test test_debc030_whatever you can run it solo with pytest -k debc030

That said, given that this involves the generator, it might be better to make a new component with required children in @plotly/dash-test-components, and test it in maybe tests/integration/test_generation?

I added debc030 for testing invalid children argument in 4800dcb

[alexcjohnson]

@siner308 @alstn2468 this is looking great. I'm not sure though why CircleCI tests don't run... normally we have no problem with PRs coming from forks, any chance there's a setting in your fork that could be blocking this? I know there's the Allow Edits from Maintainers option but even if you haven't selected that I wouldn't have expected that to matter here. @T4rk1n any ideas?

We'll want a changelog entry, which begs the question whether this is a feature addition or a bugfix. I could see calling this a bugfix, since we never said you COULDN'T set children as required so it's implied that it should work. And this has the advantage that we'd be able to release it sooner, as we'll be ready to make a patch release in the next few days but we'll want to wait a little longer before making a new minor.

[alstn2468]

@alexcjohnson I think you can check the "Build forked pull requests" option.
https://circleci.com/blog/triggering-trusted-ci-jobs-on-untrusted-forks/
https://circleci.com/docs/oss#build-pull-requests-from-forked-repositories

[alexcjohnson]

As I said, generally PRs from forks do work - see eg #2182 where a new build worked in between failed builds from this PR - so there's something else going on here.

What I see in the CircleCI project page is a bunch of messages like:

Could not find a usable config.yml, you may have revoked the CircleCI OAuth app.Please sign out of CircleCI and log back in with your VCS before triggering a new pipeline.

Which doesn't make a lot of sense given that other builds are still working fine and you didn't touch config.yml. I'll reach out to CircleCI about this, but worst case we just push this branch to the main repo and make a new PR.

[siner308]

There are more ci waiting PRs in this repository. (e.g. #1564)
Is there any commonality?

Or I can make a new PR from same source for check CI.

[alexcjohnson]

Seems to be working now, after I followed CircleCI's instructions. Hopefully it'll work for your commits too, not just the one I added by merging dev!

So now we can see a couple of tests failed: here if you can see it, both failures are in tests/unit/development/test_generate_class_file.py - probably just the tests need updating to reflect the adjusted generator, rather than a problem with the source code. The test-36 failure is spurious.

Aside from that I think we just need a changelog entry then we'll be ready to go :)

[siner308]

I just fix metadata_test.py and add changelog.
As a result, two failed test cases reported from CircleCI is passed in my local (test_class_string, test_class_file)

But CI is still not working from my commit 😥

[siner308]

I tried to setup circleci from my forked repo and pipeline works here.
But is no orb percy/agent in orbs list and percy/percy-agent repo in github is already deprecated 16 months ago.
I think this is why I failed to run ci in my forked project. Anyway, CI is triggered without problem!

I'm not sure this is a reason that cannot run in this PR.

[alexcjohnson]

💃 Looks great. Thanks again @siner308 @alstn2468! I still don't know what's going on with CircleCI and your commits, and their support folks haven't come up with any useful suggestions. But it worked when I added my own commit and all tests pass, so I think we can move on.