Skip to content

Removed buildtest build option --disable-executor-check #1541

Merged
shahzebsiddiqui merged 24 commits intobuildtesters:develfrom
Mariamajib:Mariam_patch
Jul 14, 2023
Merged

Removed buildtest build option --disable-executor-check #1541
shahzebsiddiqui merged 24 commits intobuildtesters:develfrom
Mariamajib:Mariam_patch

Conversation

@Mariamajib
Copy link
Collaborator

@Mariamajib Mariamajib commented Jul 11, 2023

No description provided.

@Mariamajib Mariamajib linked an issue Jul 11, 2023 that may be closed by this pull request
4 tasks
@Mariamajib Mariamajib self-assigned this Jul 11, 2023
@Mariamajib Mariamajib changed the title removed option --disable-executor-check from buildtest/cli/__init__.py Removed option --disable-executor-check from buildtest build Jul 11, 2023
@Mariamajib Mariamajib changed the title Removed option --disable-executor-check from buildtest build Removed buildtest build --disable-executor-check option Jul 11, 2023
@Mariamajib Mariamajib marked this pull request as ready for review July 11, 2023 20:50
@Mariamajib Mariamajib changed the title Removed buildtest build --disable-executor-check option Removed buildtest build option --disable-executor-check Jul 11, 2023
Copy link
Collaborator

@szuananwar szuananwar left a comment

Choose a reason for hiding this comment

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

@Mariamajib Please see my comments.

@szuananwar
Copy link
Collaborator

We will need to work on documenting these changes partner @Mariamajib!
@shahzebsiddiqui right?

@shahzebsiddiqui
Copy link
Member

No this doesn't need to be documented we are removing this feature

@szuananwar
Copy link
Collaborator

szuananwar commented Jul 11, 2023

@shahzebsiddiqui I meant remove it from the document too: https://buildtest.readthedocs.io/en/devel/command.html#extra

Copy link
Member

@shahzebsiddiqui shahzebsiddiqui left a comment

Choose a reason for hiding this comment

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

please make this one change you should be all set

@shahzebsiddiqui
Copy link
Member

@shahzebsiddiqui I meant remove it from the document too: https://buildtest.readthedocs.io/en/devel/command.html#extra

This is auto-generated so it will be removed, you could probably see it in the generated docs but right now they are not passing

@shahzebsiddiqui
Copy link
Member

@Mariamajib please fix the CI checks and documentation is not building

@shahzebsiddiqui
Copy link
Member

@Mariamajib i am seeing issues with regression test can you please take a look. Also documentation is not building

@shahzebsiddiqui
Copy link
Member

I did a quick search https://github.com/search?q=repo%3Abuildtesters%2Fbuildtest%20validate_executors&type=code and found a few more references to where we set validate_executors=True. Please fix this

@Mariamajib
Copy link
Collaborator Author

@shahzebsiddiqui Thank you. I will fix it!

@Mariamajib Mariamajib marked this pull request as draft July 13, 2023 15:05
@Mariamajib Mariamajib marked this pull request as ready for review July 13, 2023 15:05
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jul 13, 2023
@pull-request-size pull-request-size bot added size/S and removed size/M labels Jul 13, 2023
…norder to build

a list of valid executors. This was not invoked which caused regression test to fail because buildspecs
were not validated properly
@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Patch coverage: 66.67% and project coverage change: -0.02 ⚠️

Comparison is base (b730a97) 77.88% compared to head (eb25c46) 77.86%.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #1541      +/-   ##
==========================================
- Coverage   77.88%   77.86%   -0.02%     
==========================================
  Files          57       57              
  Lines        6623     6621       -2     
==========================================
- Hits         5158     5155       -3     
- Misses       1465     1466       +1     
Impacted Files Coverage Δ
buildtest/cli/__init__.py 100.00% <ø> (ø)
buildtest/tools/tutorialexamples.py 52.00% <0.00%> (ø)
buildtest/config.py 50.98% <100.00%> (-0.24%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

2. Made a comment in test_builders.py about a bug fixed
3. Moved test_stats.py from ../tests/cli to tests
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jul 14, 2023
@shahzebsiddiqui shahzebsiddiqui merged commit 6d2bf2d into buildtesters:devel Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove option buildtest build --disable-executor-check

3 participants