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

JS-409 CssRulingTest change mvn verify -> mvn test #4903

Merged
merged 3 commits into from
Nov 14, 2024
Merged

Conversation

zglicz
Copy link
Contributor

@zglicz zglicz commented Nov 14, 2024

JS-409

I noticed that at the end of the CssRulingTest we download some jars and try to publish a package. As @saberduck suggested, we could try to use mvn test instead of mvn verify.

@zglicz zglicz requested review from saberduck and a team November 14, 2024 13:46
@zglicz zglicz marked this pull request as draft November 14, 2024 13:46
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title CssRulingTest change mvn verify -> mvn test JS-409 CssRulingTest change mvn verify -> mvn test Nov 14, 2024
@saberduck
Copy link
Contributor

@zglicz did you check if this brings any timing benefits? I think it's quite risky change, I would keep it only if there are significant improvements and maybe let's start with Css ruling only at first

Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@zglicz
Copy link
Contributor Author

zglicz commented Nov 14, 2024

Comparing: https://github.com/SonarSource/SonarJS/runs/32957225989 (last master) and this one
CSS ruling, now: 1:23, before: 1:36
plugin qa win1, now: 12:25, before: 11:40
plugin qa win2, now: 12:40, before: 13:25
win_sonarlint: now: 7:48, before: 8:25

So I wouldn't say conclusively that it is faster, but just the fact, that we aren't trying to package and run tests - which only includes maven IntegrationTest, which as I understand, we don't use...

@zglicz zglicz marked this pull request as ready for review November 14, 2024 15:37
@saberduck saberduck merged commit 4788b03 into master Nov 14, 2024
18 of 19 checks passed
@saberduck saberduck deleted the css-ruling branch November 14, 2024 16:13
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