-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add links to the 2.3 versions of the files in the description
Please fix linting errors
Co-authored-by: Barny Shergold <barny.shergold@vaimo.com>
Hi @BarnyShergold, thanks for reviewing it. I've applied the suggestions, can you review it again? Thanks. |
@gabrieldagama - Not sure what's happened to your commits but - The 2.4 files don't seem to have all the suggested fixes applied - eg "straight forward" is one word and line 39 of static_text_execution.md needed removing. Can you please check and correct? |
@BarnyShergold yeah, I probably messed up with the commits there, my bad. I've re-applied the suggestions. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work
``` | ||
|
||
The first parameter is the mainline code without any changes, and the second parameter is the path to the folder with your changes. | ||
This command you output the results of the Semantic Version Checker tool. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do the results output to? Is it obvious?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output is the result of the CLI command in this case, I guess there is a typo there, it should be:
This command will output the results of the Semantic Version Checker tool.
Maybe we can make it more explicit by saying:
This command will output on the console the results of the Semantic Version Checker tool.
What do you think @dobooth ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The results of the Semantic Version Checker are outputted to the console.
To run static tests on all files, navigate to the Magento base directory and execute the following command: | ||
|
||
```bash | ||
bin/magento dev:test:run static |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gabrieldagama: out of curiosity, are these the exact same tests that are being ran when you submit a PR on github?
Unless something has changed recently, that's not the case. It seems like there are a bunch of extra static tests which are being executed on github.
I'm looking for easy to comprehend information about how to execute all these static tests on your local machine, it will save so much waiting time when working on a PR if we have this information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hostep, this is a valid point, thanks for bringing it up! Yes, they are the same tests, but they run a little bit differently, on our automated tests, the Static Tests are split in their separate test suite respectively (Less, HTML, PHP, Integrity...) and when you run locally, it runs all the suites at once. We should get the same results locally.
But there are somethings that I've noticed:
- Some of the tests are run based on changed files (code style, for example), so if you commit your changes before running the tests you may not get the errors.
- We do have some differences in EE and B2B versions, so if you don't have those installed you may get different results.
Maybe we can add this information to the docs as well, what do you think?
Hope this clarifies a little bit your question. If you have some PR that we can look at to see the differences it would be great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks for the info!
Ah yes, I seem to remember that it was almost always some extra tests in B2B that failed. Unfortunately we are only using Open Source so I have no access to those extra tests.
What is the reason for the static tests to be different on EE & B2B projects btw? These are static tests, in my opinion they should be 100% the same between the different type of Magento projects. I understand there is a difference for integration & functional tests, but I don't understand why there is a difference for static tests?
Would be great if the differences in static tests could get aligned so they are the same over all types of Magento projects, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you, from what I can see the PHP check are exactly the same, it just changes some files that are ignored or not. But, there are some differences in other static tests, like Integrity and Legacy tests. I'm not fully aware of exactly the reasons and what are the differences, but I may be able to check internally.
But yeah, from my point of view, ideally we would have a defined test strategy that is publicly available and applied to all versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also seem to remember vaguely that php mess detector static test ran on B2B and not on OS, but I might be mistaken
running tests |
Hi @gabrieldagama, thank you for your contribution! |
Purpose of this pull request
This pull request (PR) is to add documentation on how to run static and SVC tests.
Affected DevDocs pages
whatsnew
Added the Static Testing and Semantic Version Checker topics to the Test Guide.