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

Can we add some basic unit tests? #86

Open
jwbensley opened this issue Mar 5, 2023 · 6 comments
Open

Can we add some basic unit tests? #86

jwbensley opened this issue Mar 5, 2023 · 6 comments

Comments

@jwbensley
Copy link
Contributor

This issue is a regression bug which comes from this PR. I submitted that PR. When I wrote that PR I wanted to add some basic unit test to catch exactly that kind of simple regression bug.

One of the reasons I didn't add unit tests was because I didn't want to put too much into a single PR.

The second reason is that I don't know of a good AS or AS-SET to use.

Does anyone know of an AS and AS-SET or RouteSet which we could use to create some basic unit tests? I was thinking that it would be easy to just generate the prefixes list for a given AS-SET, in each format supported by bgpq4, save them, and then on each PR generate the each type of output again and compare to the stored "known good" reference values.

The problem with this approach is that you need an ASN and AS-SET/RouteSet which is almost never changing (I say almost because if we had to update the unit tests once every few years it wouldn't be a killer problem).

Does anyone know of an example AS-SET that exists in the IRR that is pretty much static, maybe one that is used for demonstration purposes, or one that for some reason is bound not to change?

I'm suggestion very basic tests here, but something is better than nothing, and I'd be happy to put a little PR together for this suggestion if we can agree a workable solution.

@job
Copy link
Member

job commented Mar 5, 2023

AS112 probably does not often change

@forkwhilefork
Copy link

It would also be reasonable to make your own just for these tests.

@jwbensley
Copy link
Contributor Author

Easier said that done, I don't have the means to do that, do you?

@forkwhilefork
Copy link

It takes 5 minutes...

Do you have an AltDB mntner object? If not, you'd have to create that first, otherwise just submit the objects via email once and they're created.

I would do it for you but I think it would make more sense for the objects to be owned by a bgpq4 maintainer.

@jwbensley
Copy link
Contributor Author

I'm not a maintainer, just a random person from the Internet, so I personally wouldn't create an AltDB entry for bgpq4 testing.

Maybe Job can/want's to do that, because the credentials need to be stored somewhere, which allows other maintainers to have access? But at this point I think we're over engineer a bit. my two pence is; let's just get "something" added, which can be expanded on later

@jwbensley
Copy link
Contributor Author

Added PR #87

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

No branches or pull requests

3 participants