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

feat(schema): add Pending status to JSON schema #2425

Merged
merged 14 commits into from Apr 14, 2023

Conversation

xandervedder
Copy link
Contributor

@xandervedder xandervedder commented Apr 4, 2023

Do tell if more needs to be changed or tested.

closes #2424

BREAKING CHANGE: Pending is now also a valid mutant status

See #2424 for the reasoning behind this.
@xandervedder xandervedder requested a review from nicojs April 4, 2023 12:52
Fixes build errors in places where `MutantStatus.Pending` was missing.
@hugo-vrijswijk
Copy link
Member

Could you also add the status to metrics-scala?

Copy link
Member

@nicojs nicojs left a comment

Choose a reason for hiding this comment

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

Seems like a good start. We should also update mutant-states-and-metrics.md, but that can be done in a separate PR.

I have a small remark. I'm curious what others think.

ALSO: you should add the following to your PR:

BREAKING CHANGE: 'pending' is no also a valid mutant status

packages/elements/src/lib/html-helpers.ts Outdated Show resolved Hide resolved
@nicojs
Copy link
Member

nicojs commented Apr 4, 2023

@maks-rafalko f.y.i. we're adding a pending state to a mutant as we're adding support for real-time updates to the HTML report. It's technically a breaking change since mutation testing reports that use the status won't be valid according to the v1 schema.

@maks-rafalko
Copy link

maks-rafalko commented Apr 4, 2023

thanks for letting me know. We, at Infection, don't use Pending status so I assume we are not affected by this BC.

Looking forward to this feature, to migrate to Infection.

@xandervedder
Copy link
Contributor Author

xandervedder commented Apr 6, 2023

@nicojs I'm not so sure if this is a breaking change, since I tested my (concept) implementation with Stryker.NET. It appears that the mutation report accepts a status that is not defined in the schema, in my case this was the NotRun status that Stryker.NET uses internally.

Please correct me if I'm wrong.

Edit:
It appears the validation is only done in the Stryker dashboard, so I guess it technically is a breaking change?

@rouke-broersma
Copy link
Member

rouke-broersma commented Apr 7, 2023

@xandervedder if the html report is not actually validating the received data, and instead the validation is done in the dashboard, then this is not a breaking change for the webcomponents but it is a breaking change for the schema, since the schema does enforce the status as an enum with known values. The dashboard uses the schema for validation so that makes this a breaking change in the schema. Unfortunately I think that does make this a breaking change after all. The webcomponents don't have their own versioning strategy, they are coupled to the schema.

@nicojs
Copy link
Member

nicojs commented Apr 8, 2023

F.y.i. this versioning thing is just a way to communicate to the outside world. It doesn't actually matter for us. So don't feel 'bad' about a breaking change. It's fine ☺️

@xandervedder
Copy link
Contributor Author

Is there anything else I need to change for this to go through? Happy to change anything if needed 😃

@hugo-vrijswijk
Copy link
Member

I'm thinking about the metrics. How do we want 'pending' to be calculated? I think it probably should be in the 'total' calculation

@xandervedder
Copy link
Contributor Author

I think it should because otherwise the total amount of mutants would "grow".

Also, should pending be added to the columns in the mte-metrics-table?

@rouke-broersma
Copy link
Member

I don't think pending should be in the table because it's only interesting for a very short time, and we can better use the progress bar to display this temporary state.

@xandervedder
Copy link
Contributor Author

Should I also update mutant-states-and-metrics.md? Might as well do that since it's a small change.

cc @nicojs

@nicojs
Copy link
Member

nicojs commented Apr 12, 2023

We can also document it in a separate PR IMO. Merge this? Or do a poll to decide the Emoji on the Stryker slack?

@xandervedder
Copy link
Contributor Author

xandervedder commented Apr 12, 2023

I'll make a poll in the Stryker slack (#general).

Documentation will be updated in a different PR (I'll also make an issue for that).

The user wants to see mutants if they are present in the file. To have a
filter for this would be weird, since the number in the filter would
count down as time passes.

For this reason `Pending` is not filterable.
@rouke-broersma rouke-broersma enabled auto-merge (squash) April 14, 2023 11:13
@rouke-broersma rouke-broersma merged commit c49d9a5 into master Apr 14, 2023
11 checks passed
@rouke-broersma rouke-broersma deleted the feat/mutant-status/pending branch April 14, 2023 11:24
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.

Add Pending status to mutation-testing-report-schema.json
6 participants