your-tools / tbump

Bump software releases
BSD 3-Clause "New" or "Revised" License
158 stars 22 forks source link

Thoughts about testing ... #165

Closed dmerejkowsky closed 1 year ago

dmerejkowsky commented 1 year ago

Since #164 we now has push_no_atomic in tests/project/ which is probably not what we want - after all, most devs were fine with the atomic push so far, so now our tests no longer reflect the majoriy of use case.

In insight, it's obvious we should not have duplicated the tbump.toml and test projects back when support for pyproject.toml was merged.

So we need to think of a way to test tbump with various configuration files, without duplicating them.

Also, currently we don't really check if --atomic is used during push, because we only check that the tag was created and the branch was updated.

I think this should be fixed before we make a new release - bad tests might cause bugs to occur (among other things)

dmerejkowsky commented 1 year ago

What do you think, @mlongtin0 ?

mlongtin0 commented 1 year ago

Oh yeah. I put it in one project, but not the other. But that may conflict with something else being tested.

dmerejkowsky commented 1 year ago

So ... I went ahead and did that - and I found a bug ;)

If nothing is set in the config and you use --no-atomitc-push, you still end up calling git push with the --atomic option

(see failing test in the improve-tests branch)

The problem is we call set_config in GitBumper after __init__ so the config file takes precedence over the command line (and we want the opposite)

All of this makes me wonder why we need both the config file and the command line ... Maybe we should configure the push in the config file only ?

mlongtin0 commented 1 year ago

It should probably be config only. I was following your lead with option and config.

dmerejkowsky commented 1 year ago

OK. I've opened #166