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

[BUG] Differences between GitHub comment and actual results #535

Open
6 of 9 tasks
canihavesomecoffee opened this issue Apr 27, 2021 · 8 comments
Open
6 of 9 tasks

Comments

@canihavesomecoffee
Copy link
Member

Sample platform commit (found at the bottom of each page) :

In raising this issue, I confirm the following (please check boxes, eg [X]):

  • I have read and understood the contributors guide.
  • I have checked that the bug-fix I am reporting can be replicated, or that the feature I am suggesting isn't already present.
  • I have checked that the issue I'm posting isn't already reported.
  • I have checked that the issue I'm posting isn't already solved and no duplicates exist in closed issues and in opened issues
  • I have checked the pull requests tab for existing solutions/implementations to my issue/suggestion.

My familiarity with the project is as follows (check one, eg [X]):

  • I have never visited/used the platform.
  • I have used the platform just a couple of times.
  • I have used the platform extensively, but have not contributed previously.
  • I am an active contributor to the platform.

The posted results to a PR on GitHub are inconsistent with what you see when you visit the actual results page.

For example:

CCExtractor/ccextractor#1329 (comment) (table mentions fails, below it says all tests passed)
vs
https://sampleplatform.ccextractor.org/test/3224 (all tests passed)

Another example:
CCExtractor/ccextractor#1329 (comment) (~19 listed failures)
vs
https://sampleplatform.ccextractor.org/test/3225 (5 fails)

@pranavrajpal
Copy link
Contributor

I'd like to try working on this.

Is there a way to download the server's database so that I can try and look at the test results more closely? Right now, I can't really tell whether the correct test results are the ones shown on the Sample Platform website, the ones shown in the GitHub comment, or neither of them.

@kvshravan
Copy link
Contributor

@pranavrajpal I don't think you'll be needing the server database, sample database which comes through the installation must be enough, I suppose the issue is with the logic of comment_pr function, especially while counting for the passed tests.

@canihavesomecoffee
Copy link
Member Author

The correct results are on the page, so it's indeed the commenting function logic which goes wrong for some reason.

@kvshravan
Copy link
Contributor

@pranavrajpal Are you still working on this? If you're stuck somewhere , we're ready to help :)

@pranavrajpal
Copy link
Contributor

Yes, I'm still working on this.

Currently, I'm still trying to determine what the cause of this is. I think that the problem is somehow related to regression tests that have multiple expected variants of a single output file, because several of the regression tests I've found that are incorrectly marked as failing in the comment in CCExtractor/ccextractor#1329 seem to have 2 variants for output file 1 (so far I've found 2, 19, and 104). However, I'm still trying to reproduce this so I can't say for certain.

I have a question about how regression tests with multiple variants of the same file (like the previously mentioned tests) are stored in the database. The relevant model definitions for how the output files are stored seem to be RegressionTestOutput:

class RegressionTestOutput(Base):
"""Model to store output of regression test."""
__tablename__ = 'regression_test_output'
__table_args__ = {'mysql_engine': 'InnoDB'}
id = Column(Integer, primary_key=True)
regression_id = Column(Integer, ForeignKey('regression_test.id', onupdate='CASCADE', ondelete='RESTRICT'))
regression_test = relationship('RegressionTest', back_populates='output_files')
correct = Column(Text())
correct_extension = Column(String(64), nullable=False) # contains the .
expected_filename = Column(Text())
ignore = Column(Boolean(), default=False)
multiple_files = relationship('RegressionTestOutputFiles', back_populates='output')

and RegressionTestOutputFiles:
class RegressionTestOutputFiles(Base):
"""Model to store multiple correct output files for a regression_test."""
__tablename__ = 'regression_test_output_files'
__table_args__ = {'mysql_engine': 'InnoDB'}
id = Column(Integer, primary_key=True)
file_hashes = Column(Text())
regression_test_output_id = Column(
Integer,
ForeignKey('regression_test_output.id', onupdate='CASCADE', ondelete='RESTRICT')
)
output = relationship('RegressionTestOutput', back_populates='multiple_files')

If there are multiple allowed variants of an output file, would that be represented as:

  • a RegressionTestOutput with correct describing the original version of the file and multiple_files describing the variants?
  • a RegressionTestOutput with multiple_files describing both the original version and the variants of the output files and correct being ignored?
  • multiple RegressionTestOutput instances all pointing to the same regression test (either using regression_test or regression_id)?
  • something else entirely?

@kvshravan
Copy link
Contributor

kvshravan commented May 7, 2021

Yes , you're correct. RegressionTestOutput model is used to store the original version of the output and multiple_files will fetch us the variant files. The testsuite checks if the output for a particular regression test matches with the original output. If the output matches with the original file, the got field will be set to None, else the got field will contain hash of the output file. A test is passed if got is None or if the hash value of got is one of the multiple_files. You can refer to Pass/fail mechanism in mod_test/controllers.py. This should help you deal with the issue.

@devmrfitz
Copy link
Contributor

@canihavesomecoffee Is this issue fixed or still open?

@thealphadollar
Copy link
Contributor

Hey @devmrfitz , only part of the issue has been solved by #540 . Please refer the PR to know more :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants